Unlocked pages flushed

Unlocked pages flushed

[Rajat]

Hi List,

While flushing the data through nfs_file_flush, I observed that pages are 
being flushed in unlocked state. nfs_file_flush call graph reaches to 
nfs_flush_one where the comment above this definition states "The page must 
have been locked by the caller." However I have checked with PageLocked() call 
that the pages which enter into nfs_flush_one are not locked. 

What I could make out of it is: this is done porbably to avoid deadlock with 
other functions like prepare_write, commit_write, writepage, readpage where 
page is grabbed first then lock_kernel (HEAVILY used in NFS) is called. 
Whereas nfs_file_flush calls lock_kernel, then enters into nfs_wb_all. So, to 
avoid deadlock, logical place for lock_page loop would have been in 
nfs_file_flush before lock_kernel (still not sure about this, will it help?). 

Here we are flushing pages without honouring page_lock OR am I going wrong 
somewhere???

Rajat


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Unlocked pages flushed

[Trond Myklebust]

On Mon, 2007-07-02 at 10:51 +0000, Rajat wrote:
> Hi List,
> 
> While flushing the data through nfs_file_flush, I observed that pages are 
> being flushed in unlocked state. nfs_file_flush call graph reaches to 
> nfs_flush_one where the comment above this definition states "The page must 
> have been locked by the caller." However I have checked with PageLocked() call 
> that the pages which enter into nfs_flush_one are not locked. 
> 
> What I could make out of it is: this is done porbably to avoid deadlock with 
> other functions like prepare_write, commit_write, writepage, readpage where 
> page is grabbed first then lock_kernel (HEAVILY used in NFS) is called. 
> Whereas nfs_file_flush calls lock_kernel, then enters into nfs_wb_all. So, to 
> avoid deadlock, logical place for lock_page loop would have been in 
> nfs_file_flush before lock_kernel (still not sure about this, will it help?). 
> 
> Here we are flushing pages without honouring page_lock OR am I going wrong 
> somewhere???
> 
> Rajat

When you hit contention on a mutex or page lock, that puts the process
to sleep: at that point the BKL is automatically released, and so no
deadlock occurs.

IOW: it is impossible to get the BKL to ABBA deadlock with any form of
"sleeping lock".

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Unlocked pages flushed

[Rajat]

Trond Myklebust <trond.myklebust <at> fys.uio.no> writes:

> 
> On Mon, 2007-07-02 at 10:51 +0000, Rajat wrote:
> > Hi List,
> > 
> > While flushing the data through nfs_file_flush, I observed that pages are 
> > being flushed in unlocked state. nfs_file_flush call graph reaches to 
> > nfs_flush_one where the comment above this definition states "The page 
must 
> > have been locked by the caller." However I have checked with PageLocked() 
call 
> > that the pages which enter into nfs_flush_one are not locked. 
> > 
> > What I could make out of it is: this is done porbably to avoid deadlock 
with 
> > other functions like prepare_write, commit_write, writepage, readpage 
where 
> > page is grabbed first then lock_kernel (HEAVILY used in NFS) is called. 
> > Whereas nfs_file_flush calls lock_kernel, then enters into nfs_wb_all. So, 
to 
> > avoid deadlock, logical place for lock_page loop would have been in 
> > nfs_file_flush before lock_kernel (still not sure about this, will it 
help?). 
> > 
> > Here we are flushing pages without honouring page_lock OR am I going wrong 
> > somewhere???
> > 
> > Rajat
> 
> When you hit contention on a mutex or page lock, that puts the process
> to sleep: at that point the BKL is automatically released, and so no
> deadlock occurs.
> 
> IOW: it is impossible to get the BKL to ABBA deadlock with any form of
> "sleeping lock".

Yes I missed out on this. page lock is sleeping on bit. But then if ABBA 
deadlock is not the case, why pages are being flushed unlocked? I mean is it 
not required to lock these pages before flushing? In that case it should be 
safe to place page_lock loop to wait on any pending work on these pages. 

Also there is one more spinlock in the scene, nfs_inode->req_lock and spinning 
for this lock should not release BKL. I am assuming that lock ordering of BKL 
and req_lock is maintained. 



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Unlocked pages flushed

[Trond Myklebust]

On Mon, 2007-07-02 at 13:24 +0000, Rajat wrote:
> Yes I missed out on this. page lock is sleeping on bit. But then if ABBA 
> deadlock is not the case, why pages are being flushed unlocked? I mean is it 
> not required to lock these pages before flushing? In that case it should be 
> safe to place page_lock loop to wait on any pending work on these pages. 

The only VM requirement is that the pages be locked when you set the
PG_writeback flag. This is already the case in recent kernels.

Otherwise, NFS manages its own locking scheme on the nfs_page.

> Also there is one more spinlock in the scene, nfs_inode->req_lock and spinning 
> for this lock should not release BKL. I am assuming that lock ordering of BKL 
> and req_lock is maintained. 

Whenever necessary, the BKL is taken before we take the req_lock. There
are AFAIK no cases where we take the BKL while we are holding the
req_lock.

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Unlocked pages flushed

[Rajat]

> The only VM requirement is that the pages be locked when you set the
> PG_writeback flag. This is already the case in recent kernels.

Oh! I didnt know that.   

> Otherwise, NFS manages its own locking scheme on the nfs_page.

Now that makes sense. Thanks a lot Trond.

Rajat


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs