Re: [GIT PULL] Please pull NFS client changes for 4.18

From: NeilBrown <neilb@suse.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	trondmy@hammerspace.com,
	Paul McKenney <paulmck@linux.vnet.ibm.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [GIT PULL] Please pull NFS client changes for 4.18
Date: Wed, 13 Jun 2018 08:01:58 +1000	[thread overview]
Message-ID: <87vaanirzd.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <CA+55aFz3jmGST_cOB+qH6qZTFa5HJcw2JoeWxJEZdWgeO4=vNQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]

On Tue, Jun 12 2018, Linus Torvalds wrote:

> Final note (for now) on this: I've merged the nfs code, but I really
> am obviously not happy with these crazy random ad-hoc
> cursor-not-cursor list games.
>
>                 Linus
Hi Linus,
 thanks for merging the code despite your reservations.

 Yes, we could create a generic rcu-list cursor.  I have given it some
 thought but didn't like the added complexity.  As there were existing
 objects in the list that could be used as a cursor, that seemed to me to
 be the better solution.

 As you say, and cursor would need to be allocated from a slab, not on
 the stack. We could use a SLAB_TYPESAFE_BY_RCU and not need to use rcu
 to delay the freeing.
 The lsb in the next pointer of the cursor would be 1 to indicate the
 cursor.

 Any iteration of the list would need to look out for this flag.
 When found it would need to skip over any cursors to the next
 non-cursor, then repeat the skip and make sure it found the same
 non-cursor.  This guards against the cursor moving while it is being
 examined.

 Any walk that needed to place a cursor would need to get an exclusive
 lock on the list from the start.  This is more locking overhead than
 just grabbing the lock to optimistically take a reference on the
 "current" item which I did in the NFS patch.  If the lists were
 normally short that might not be a problem. In this case the list can
 get quite long so the extra locking might be noticeable.

 Deleting objects from the list would need to be careful to preserve the
 flag bit, but that is the least difficult part.

 FYI I have an open proposal to improve the cursor used by rhashtable
 for rhashtable_walk - it sometimes needs to drop out of RCU in the
 middle of a bucket chain.  In that case the chain is normally short (16 is
 considered so long that the hash must have been compromised) and I
 propose an insertion sort to keep the addresses of objects in numerical
 order.  This way the address of the last object found can work as a stable
 cursor - we just search through the list until an object has a larger
 address.

 So my perspective is that while an rcu_cursor_list could be developed,
 I'm not sure it would always (or ever?) be the best solution to a
 given problem.
 I can turn these thoughts into a patch if you like and see what people
 think.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]