Skip to contentSkip to conversation

Eliminating the BKL from the NFS and RPC clients

DISCUSSION Chuck Lever 3 participants 5 messages Jan 18, 2007 – Jan 24, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi all-

I'd like some wider review of the series of patches that eliminate the BKL from the NFS and RPC client. There are quite a few of them, so instead of spamming the list, here's the URL:

http://oss.oracle.com/~cel/linux-2.6/2.6.20-rc4/patches

The BKL patches are numbered 052 through 138. I know there are a lot of these, but each is tiny, so we can use 'git bisect' easily to narrow down a problematic case. In addition, they are, by and large, not interdependent (although order is important) so you can apply a few of them from the series at a time, if that is desired.

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture Linux Projects Group email;internet:chuck dot lever at nospam oracle dot com title:Principle Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#1 lore raw

056-nfs-smp-change_attribute.diff:
are the barriers really enough? Don't you need at least a lock on the writer side?

057-noattr_flag.diff:
I don't really like this approach. Can we make touch_atime and file_update_time go ghrough ->setattr instead? Having all attribute updates going through setattr would fix up quite a lot of mess we have, also in other filesystems like xfs. In either case this really needs to go through -fsdevel

059-nfs-serialize-change_attr.diff:
Please don't spread all these CONFIG_NFS_V4 around, they don't really seem to help much.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#2 lore raw
Quoted 4 lines · Christoph Hellwig

056-nfs-smp-change_attribute.diff:
are the barriers really enough? Don't you need at least a lock on the writer side?

The change attribute should always be updated under the inode->lock anyway. I'm not really sure we need these.

Quoted 6 lines

057-noattr_flag.diff:
I don't really like this approach. Can we make touch_atime and file_update_time go ghrough ->setattr instead? Having all attribute updates going through setattr would fix up quite a lot of mess we have, also in other filesystems like xfs. In either case this really needs to go through -fsdevel

No. Attributes may change at any moment on the server. That is _very_ different to the case where the user or the client changes the attributes via setattr. For one thing, we don't have to notify the server. Secondly, we do want to be able to update those attributes from asynchronous contexts (for instance when a WRITE request returns with updated).
The client should in any case _never_ be overriding the server attributes. The only exception to that rule is if we have cached writes which change the file size. We do try to detect that...

Quoted 3 lines

059-nfs-serialize-change_attr.diff:
Please don't spread all these CONFIG_NFS_V4 around, they don't really seem to help much.

ACK.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#3 lore raw
Quoted 9 lines · Trond Myklebust

No. Attributes may change at any moment on the server. That is _very_ different to the case where the user or the client changes the attributes via setattr. For one thing, we don't have to notify the server. Secondly, we do want to be able to update those attributes from asynchronous contexts (for instance when a WRITE request returns with updated).
The client should in any case _never_ be overriding the server attributes. The only exception to that rule is if we have cached writes which change the file size. We do try to detect that...

Sorry, my previous mail wasn't clear enough on how I want setattr used here. I think it's right to make all these attribute updates through ->setattr so filesystem can deal with them explicitly. Of course we'd still need to treat explicit updates different from implicit one, probably by adding in ATTR_IMPLIZIT for the later.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#4 lore raw
In reply to Trond Myklebust
Quoted 7 lines · Trond Myklebust
Quoted 3 lines · Christoph Hellwig

056-nfs-smp-change_attribute.diff:
are the barriers really enough? Don't you need at least a lock on the writer side?

The change attribute should always be updated under the inode->lock anyway. I'm not really sure we need these.

What about nfs_end_data_update ?

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture Linux Projects Group email;internet:chuck dot lever at nospam oracle dot com title:Principle Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs