xfs
[Top] [All Lists]

Re: [PATCH] Remove DIO_OWN_LOCKING

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [PATCH] Remove DIO_OWN_LOCKING
From: Russell Cattelan <cattelan@xxxxxxxxxxx>
Date: Fri, 13 Oct 2006 13:09:33 -0500
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <20061013024830.GF11034@xxxxxxxxxxxxxxxxx>
References: <1160700998.5723.65.camel@xxxxxxxxxxxxxxxxxxxx> <20061013024830.GF11034@xxxxxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
On Fri, 2006-10-13 at 12:48 +1000, David Chinner wrote:
> On Thu, Oct 12, 2006 at 07:56:38PM -0500, Russell Cattelan wrote:
> > While trying to fix up GFS2 directio and reading through the code
> > involving the various lock flags I discovered the DIO_OWN_LOCKING 
> > flag is no longer used.
> >  
> > XFS recently changed it xfs_vm_direct_IO function to call
> > blockdev_direct_IO_no_locking for reads and
> > blockdev_direct_IO_own_locking
> > for writes. But DIO_OWN_LOCKING is only used in the direct IO read case
> > so effectively the flag is never checked an therefore can probably be
> > removed.
> 
> NACK.
> 
> This breaks XFS direct writes - the DIO_OWN_LOCKING flag has meaning
> for direct writes even though a simple grep doesn't give you any
> hits. get_more_blocks() sets the create flag unconditionally on
> writes when DIO_OWN_LOCKING is set, and this is needed for XFS to be
> able to allocate underlying blocks if the direct write is over a
> hole or past EOF.
Arrghh you are correct!
Even more reason to clean this logic up.

look this version over and see what you think.

comments not in final state but is describing what
is being changed an why.

Basically the idea is to have separate flags for locking
and creation, overloading the flags meant that they were
specific for XFS needs and therefore did not work for
GFS.

Also go to a TRUE state if flag on and a FALSE state if flag off.
vs the mix of true flag (DIO_LOCKING) vs false flag
(DIO_NO_LOCKING)


The other option might be to have xfs pass different get_blocks
functions
for read and write, but that seems even more confusing based on the
presence
of create flag.
And probably wouldn't help GFS in the long run unless it did the same 
get_block_read/get_block_write thing.



-- 
Russell Cattelan <cattelan@xxxxxxxxxxx>

Attachment: wack_own_locking
Description: Text Data

Attachment: signature.asc
Description: This is a digitally signed message part

<Prev in Thread] Current Thread [Next in Thread>