2018-06-12 13:40:09

by Trond Myklebust

[permalink] [raw]
Subject: [GIT PULL] Please pull NFS client changes for 4.18

Hi Linus,

The following changes since commit 786b71f5b754273ccef6d9462e52062b3e1f9877:

Merge tag 'nds32-for-linus-4.17-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/greentime/linux (2018-05-28 05:25:57 -0700)

are available in the Git repository at:

git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-4.18-1

for you to fetch changes up to 93b7f7ad2018d2037559b1d0892417864c78b371:

skip LAYOUTRETURN if layout is invalid (2018-06-12 08:48:04 -0400)

Cheers,
Trond

----------------------------------------------------------------
NFS client updates for Linux 4.18

Highlights include:

Stable fixes:
- Fix a 1-byte stack overflow in nfs_idmap_read_and_verify_message
- Fix a hang due to incorrect error returns in rpcrdma_convert_iovs()
- Revert an incorrect change to the NFSv4.1 callback channel
- Fix a bug in the NFSv4.1 sequence error handling

Features and optimisations:
- Support for piggybacking a LAYOUTGET operation to the OPEN compound
- RDMA performance enhancements to deal with transport congestion
- Add proper SPDX tags for NetApp-contributed RDMA source
- Do not request delegated file attributes (size+change) from the server
- Optimise away a GETATTR in the lookup revalidate code when doing NFSv4 OPEN
- Optimise away unnecessary lookups for rename targets
- Misc performance improvements when freeing NFSv4 delegations

Bugfixes and cleanups:
- Try to fail quickly if proto=rdma
- Clean up RDMA receive trace points
- Fix sillyrename to return the delegation when appropriate
- Misc attribute revalidation fixes
- Immediately clear the pNFS layout on a file when the server returns ESTALE
- Return NFS4ERR_DELAY when delegation/layout recalls fail due to igrab()
- Fix the client behaviour on NFS4ERR_SEQ_FALSE_RETRY

----------------------------------------------------------------
Anna Schumaker (4):
NFS: Move call to nfs4_state_protect_write() to nfs4_write_setup()
NFS: Move call to nfs4_state_protect() to nfs4_commit_setup()
NFS: Pass "privileged" value to nfs4_init_sequence()
NFS: Merge nfs41_free_stateid() with _nfs41_free_stateid()

Benjamin Coddington (1):
NFSv4: Don't add a new lock on an interrupted wait for LOCK

Chuck Lever (21):
xprtrdma: Add proper SPDX tags for NetApp-contributed source
xprtrdma: Try to fail quickly if proto=rdma
xprtrdma: Create transport's CM ID in the correct network namespace
xprtrdma: Fix max_send_wr computation
SUNRPC: Initialize rpc_rqst outside of xprt->reserve_lock
SUNRPC: Add a ->free_slot transport callout
xprtrdma: Introduce ->alloc_slot call-out for xprtrdma
xprtrdma: Make rpc_rqst part of rpcrdma_req
xprtrdma: Clean up Receive trace points
xprtrdma: Move Receive posting to Receive handler
xprtrdma: Remove rpcrdma_ep_{post_recv, post_extra_recv}
xprtrdma: Remove rpcrdma_buffer_get_req_locked()
xprtrdma: Remove rpcrdma_buffer_get_rep_locked()
xprtrdma: Make rpcrdma_sendctx_put_locked() a static function
xprtrdma: Return -ENOBUFS when no pages are available
xprtrdma: Move common wait_for_buffer_space call to parent function
xprtrdma: Wait on empty sendctx queue
xprtrdma: Add trace_xprtrdma_dma_map(mr)
xprtrdma: Remove transfertypes array
NFSv4.0: Remove cl_ipaddr from non-UCS client ID
NFSv4.0: Remove transport protocol name from non-UCS client ID

Dave Wysochanski (1):
NFSv4: Fix possible 1-byte stack overflow in nfs_idmap_read_and_verify_message

Fred Isaman (14):
pnfs: Remove redundant assignment from nfs4_proc_layoutget().
pnfs: Store return value of decode_layoutget for later processing
NFS4: move ctx into nfs4_run_open_task
pnfs: Add layout driver flag PNFS_LAYOUTGET_ON_OPEN
pnfs: refactor send_layoutget
pnfs: move allocations out of nfs4_proc_layoutget
pnfs: Add conditional encode/decode of LAYOUTGET within OPEN compound
pnfs: Move nfs4_opendata into nfs4_fs.h
pnfs: Change pnfs_alloc_init_layoutget_args call signature
pnfs: Add LAYOUTGET to OPEN of a new file
pnfs: Add LAYOUTGET to OPEN of an existing file
pnfs: Stop attempting LAYOUTGET on OPEN on failure
pnfs: Add barrier to prevent lgopen using LAYOUTGET during recall
pnfs: Fix manipulation of NFS_LAYOUT_FIRST_LAYOUTGET

NeilBrown (4):
NFS: slight optimization for walking list for delegations
NFS: use cond_resched() when restarting walk of delegation list.
rculist: add list_for_each_entry_from_rcu()
NFS: Avoid quadratic search when freeing delegations.

Olga Kornievskaia (1):
skip LAYOUTRETURN if layout is invalid

Trond Myklebust (35):
NFS: Optimise away the close-to-open GETATTR when we have NFSv4 OPEN
NFS: If the VFS sets LOOKUP_REVAL then force a lookup of the dentry
NFS: Optimise away lookups for rename targets
NFSv4: Only pass the delegation to setattr if we're sending a truncate
NFSv4: Fix sillyrename to return the delegation when appropriate
NFS: Fix up sillyrename()
NFS: Set the force revalidate flag if the inode is not completely initialised
NFS: Ensure we revalidate the inode correctly after remove or rename
NFS: Ensure we revalidate the inode correctly after setacl
NFS: Fix up nfs_post_op_update_inode() to force ctime updates
NFSv4: Always clear the pNFS layout when handling ESTALE
pNFS: Refactor nfs4_layoutget_release()
NFSv4/pnfs: Ensure pnfs_parse_lgopen() won't try to parse uninitialised data
NFSv4/pnfs: Don't switch off layoutget-on-open for transient errors
pNFS: Don't send LAYOUTGET on OPEN for read, if we already have cached data
pnfs: Don't call commit on failed layoutget-on-open
pnfs: Don't release the sequence slot until we've processed layoutget on open
NFSv4: Don't request size+change attribute if they are delegated to us
NFS: Pass the inode down to the getattr() callback
NFSv4: Don't ask for delegated attributes when revalidating the inode
NFSv4: Don't ask for delegated attributes when adding a hard link
NFSv4: Ignore NFS_INO_REVAL_FORCED in nfs4_proc_access
NFSv4: Ensure the inode is clean when we set a delegation
NFS: fix up nfs_setattr_update_inode
NFS: Fix attribute revalidation
NFS: Improve caching while holding a delegation
NFS: Ignore NFS_INO_REVAL_FORCED in nfs_check_inode_attributes()
NFS: Filter cache invalidation when holding a delegation
Merge tag 'nfs-rdma-for-4.18-1' of git://git.linux-nfs.org/projects/anna/linux-nfs
NFSv4: Fix a compiler warning when CONFIG_NFS_V4_1 is undefined
NFSv4: Return NFS4ERR_DELAY when a delegation recall fails due to igrab()
NFSv4: Return NFS4ERR_DELAY when a layout recall fails due to igrab()
NFSv4: Revert commit 5f83d86cf531d ("NFSv4.x: Fix wraparound issues..")
NFSv4: Fix a typo in nfs41_sequence_process
NFSv4.1: Fix the client behaviour on NFS4ERR_SEQ_FALSE_RETRY

fs/nfs/callback_proc.c | 43 ++--
fs/nfs/client.c | 3 +-
fs/nfs/delegation.c | 86 +++++--
fs/nfs/dir.c | 51 +++-
fs/nfs/export.c | 2 +-
fs/nfs/flexfilelayout/flexfilelayout.c | 1 +
fs/nfs/inode.c | 126 +++++++---
fs/nfs/nfs3proc.c | 13 +-
fs/nfs/nfs42proc.c | 6 +-
fs/nfs/nfs4_fs.h | 27 +-
fs/nfs/nfs4idmap.c | 5 +-
fs/nfs/nfs4proc.c | 391 +++++++++++++----------------
fs/nfs/nfs4state.c | 8 +
fs/nfs/nfs4xdr.c | 65 ++++-
fs/nfs/pnfs.c | 331 +++++++++++++++++++++---
fs/nfs/pnfs.h | 28 ++-
fs/nfs/proc.c | 13 +-
fs/nfs/unlink.c | 20 +-
fs/nfs/write.c | 10 +-
include/linux/nfs_fs_sb.h | 2 +
include/linux/nfs_xdr.h | 15 +-
include/linux/rculist.h | 13 +
include/linux/sunrpc/rpc_rdma.h | 1 +
include/linux/sunrpc/xprt.h | 6 +-
include/linux/sunrpc/xprtrdma.h | 1 +
include/trace/events/rpcrdma.h | 76 ++++--
net/sunrpc/clnt.c | 1 +
net/sunrpc/xprt.c | 17 +-
net/sunrpc/xprtrdma/backchannel.c | 105 +++-----
net/sunrpc/xprtrdma/fmr_ops.c | 23 ++
net/sunrpc/xprtrdma/frwr_ops.c | 31 ++-
net/sunrpc/xprtrdma/module.c | 1 +
net/sunrpc/xprtrdma/rpc_rdma.c | 66 ++---
net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 +
net/sunrpc/xprtrdma/transport.c | 64 +++--
net/sunrpc/xprtrdma/verbs.c | 291 +++++++++------------
net/sunrpc/xprtrdma/xprt_rdma.h | 26 +-
net/sunrpc/xprtsock.c | 4 +
38 files changed, 1240 insertions(+), 733 deletions(-)
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2018-06-12 17:44:48

by Linus Torvalds

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

On Tue, Jun 12, 2018 at 6:39 AM Trond Myklebust <[email protected]> wrote:
>
> NeilBrown (4):
> rculist: add list_for_each_entry_from_rcu()

Oh christ, people. Not another one of these.

We need to start having a real rule in place where people DO NOT PLAY
GAMES WITH RCU LISTS.

Adding Paul McKenney to the list, since he clearly wasn't for the
actual development, and the commit has no sign that it was sanity
checked.

This whole "let's re-start RCU walking in the middle after we've
dropped the RCU read lock" needs a hell of a lot more thought than
people seem to actually be giving it.

Maybe the code is actually correct - it looks ok to me. But every time
I see it I just shudder.

What people actually want is to just have a cursor entry in an RCU
list. I'd much rather have *that*, than have people make up these
insane ad-hoc cursor entries that are insane and have horrible
semantics.

For example, looking at that commit e04bbf6b1bbe ("NFS: Avoid
quadratic search when freeing delegations"), it even talks about the
ad-hoc cursor entry not actually being reliable, and how that's ok
because the code doesn't care. No, random odd behavior that is
basically never ever going to be testable is *NOT* ok.

So could we please have a cursor entry for RCU walking, and actually
have a agreed-upon way to do this? Maybe even using the low bit in the
"next" field to mark a cursor entry generically - the same way we
already do for the HEAD field in the bit-locked lists, just on the
actual entries _on_ the list instead.

Because we now have *two* of these odd special "restart the loop in
the middle" come in during the same merge window. That really implies
to me that we should have a _proper_ solution for the problem, not
this horribly nasty case where people make up their own pseudo-cursors
that have odd and questionable semantics.

Paul, comments?

Linus

2018-06-12 18:07:35

by Linus Torvalds

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

On Tue, Jun 12, 2018 at 10:42 AM Linus Torvalds
<[email protected]> wrote:
>
> So could we please have a cursor entry for RCU walking, and actually
> have a agreed-upon way to do this? Maybe even using the low bit in the
> "next" field to mark a cursor entry generically - the same way we
> already do for the HEAD field in the bit-locked lists, just on the
> actual entries _on_ the list instead.

The real problem with a RCU cursor is that the lifetime of the cursor
is also RCU extended, so you can't do the traditional "just allocate
the cursor entry on the stack" that you can with synchronous locked
lists.

That makes it slightly more inconvenient to do simple cursors.

The dcache code has a fairly clean "dcache cursor", but it does use a
whole "struct dentry" for it (and it depends on "next_positive()" to
just skip them because dursors are never _positive_ dentries, so
there's some subtlety there).

But at least it's a very explicit cursor model, not that odd
delegation inode that seems to have a life as a real inode too.

So maybe a generic rcu-list cursor is too hard to do sanely, but can
we at least encourage that very explicit cursor model that is *only* a
cursor, and might not be touched/moved/reallocated by something else?

Linus

2018-06-12 18:10:02

by Linus Torvalds

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

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

2018-06-12 22:03:34

by NeilBrown

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

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


Attachments:
signature.asc (847.00 B)

2018-06-13 01:03:50

by Paul E. McKenney

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

On Tue, Jun 12, 2018 at 11:08:31AM -0700, 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.

We did review this one, and Neil did make requested changes to the comment
header and documentation. However, as you say, I will push back harder
on future RCU list traversal macros.

Thanx, Paul


2018-06-13 01:12:45

by Linus Torvalds

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

On Tue, Jun 12, 2018 at 6:02 PM Paul E. McKenney
<[email protected]> wrote:
>
> We did review this one, and Neil did make requested changes to the comment
> header and documentation.

Oh, I checked the commit for "acked-by" from you and didn't see any,
so I was assuming this was just Neil and Trond on their own..

Linus

2018-06-13 01:38:23

by Paul E. McKenney

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

On Tue, Jun 12, 2018 at 06:11:50PM -0700, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 6:02 PM Paul E. McKenney
> <[email protected]> wrote:
> >
> > We did review this one, and Neil did make requested changes to the comment
> > header and documentation.
>
> Oh, I checked the commit for "acked-by" from you and didn't see any,
> so I was assuming this was just Neil and Trond on their own..

Hmmm... I gave them a Reviewed-by, but perhaps it got lost.

http://lkml.kernel.org/r/[email protected]

Thanx, Paul


2018-06-13 01:43:42

by NeilBrown

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

On Tue, Jun 12 2018, Paul E. McKenney wrote:

> On Tue, Jun 12, 2018 at 06:11:50PM -0700, Linus Torvalds wrote:
>> On Tue, Jun 12, 2018 at 6:02 PM Paul E. McKenney
>> <[email protected]> wrote:
>> >
>> > We did review this one, and Neil did make requested changes to the comment
>> > header and documentation.
>>
>> Oh, I checked the commit for "acked-by" from you and didn't see any,
>> so I was assuming this was just Neil and Trond on their own..
>
> Hmmm... I gave them a Reviewed-by, but perhaps it got lost.
>
> http://lkml.kernel.org/r/[email protected]
>
> Thanx, Paul

Sorry about that. I think I planned to add it after I heard back from
Trond what he thought of the patch, but I didn't hear anything until it
landed. I'll try to be more proactive next time.

Thanks,
NeilBrown


Attachments:
signature.asc (847.00 B)

2018-06-13 10:06:44

by Paul E. McKenney

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

On Wed, Jun 13, 2018 at 11:42:51AM +1000, NeilBrown wrote:
> On Tue, Jun 12 2018, Paul E. McKenney wrote:
>
> > On Tue, Jun 12, 2018 at 06:11:50PM -0700, Linus Torvalds wrote:
> >> On Tue, Jun 12, 2018 at 6:02 PM Paul E. McKenney
> >> <[email protected]> wrote:
> >> >
> >> > We did review this one, and Neil did make requested changes to the comment
> >> > header and documentation.
> >>
> >> Oh, I checked the commit for "acked-by" from you and didn't see any,
> >> so I was assuming this was just Neil and Trond on their own..
> >
> > Hmmm... I gave them a Reviewed-by, but perhaps it got lost.
> >
> > http://lkml.kernel.org/r/[email protected]
>
> Sorry about that. I think I planned to add it after I heard back from
> Trond what he thought of the patch, but I didn't hear anything until it
> landed. I'll try to be more proactive next time.

Not a problem from my viewpoint. There may come a time when I need to
care about contribution metrics, but I don't need to at the moment. ;-)

Thanx, Paul


2018-06-18 04:23:45

by NeilBrown

[permalink] [raw]
Subject: [PATCH] rculist: improve documentation for list_for_each_entry_from_rcu()


Unfortunately the patch for adding list_for_each_entry_from_rcu()
wasn't the final patch after all review. It is functionally
correct but the documentation was incomplete.

This patch adds this missing documentation which includes an update to
the documentation for list_for_each_entry_continue_rcu() to match the
documentation for the new list_for_each_entry_from_rcu(), and adds
list_for_each_entry_from_rcu() and the already existing
hlist_for_each_entry_from_rcu() to section 7 of whatisRCU.txt.

Reviewed-by: Paul E. McKenney <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
Documentation/RCU/whatisRCU.txt | 2 ++
include/linux/rculist.h | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
index 65eb856526b7..d43524493fb3 100644
--- a/Documentation/RCU/whatisRCU.txt
+++ b/Documentation/RCU/whatisRCU.txt
@@ -816,11 +816,13 @@ RCU list traversal:
list_next_rcu
list_for_each_entry_rcu
list_for_each_entry_continue_rcu
+ list_for_each_entry_from_rcu
hlist_first_rcu
hlist_next_rcu
hlist_pprev_rcu
hlist_for_each_entry_rcu
hlist_for_each_entry_rcu_bh
+ hlist_for_each_entry_from_rcu
hlist_for_each_entry_continue_rcu
hlist_for_each_entry_continue_rcu_bh
hlist_nulls_first_rcu
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 36df6ccbc874..4786c2235b98 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -396,7 +396,16 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
* @member: the name of the list_head within the struct.
*
* Continue to iterate over list of given type, continuing after
- * the current position.
+ * the current position which must have been in the list when the RCU read
+ * lock was taken.
+ * This would typically require either that you obtained the node from a
+ * previous walk of the list in the same RCU read-side critical section, or
+ * that you held some sort of non-RCU reference (such as a reference count)
+ * to keep the node alive *and* in the list.
+ *
+ * This iterator is similar to list_for_each_entry_from_rcu() except
+ * this starts after the given position and that one starts at the given
+ * position.
*/
#define list_for_each_entry_continue_rcu(pos, head, member) \
for (pos = list_entry_rcu(pos->member.next, typeof(*pos), member); \
@@ -411,6 +420,14 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
*
* Iterate over the tail of a list starting from a given position,
* which must have been in the list when the RCU read lock was taken.
+ * This would typically require either that you obtained the node from a
+ * previous walk of the list in the same RCU read-side critical section, or
+ * that you held some sort of non-RCU reference (such as a reference count)
+ * to keep the node alive *and* in the list.
+ *
+ * This iterator is similar to list_for_each_entry_continue_rcu() except
+ * this starts from the given position and that one starts from the position
+ * after the given position.
*/
#define list_for_each_entry_from_rcu(pos, head, member) \
for (; &(pos)->member != (head); \
--
2.14.0.rc0.dirty


Attachments:
signature.asc (847.00 B)

2018-06-18 17:01:15

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rculist: improve documentation for list_for_each_entry_from_rcu()

On Mon, Jun 18, 2018 at 02:22:40PM +1000, NeilBrown wrote:
>
> Unfortunately the patch for adding list_for_each_entry_from_rcu()
> wasn't the final patch after all review. It is functionally
> correct but the documentation was incomplete.
>
> This patch adds this missing documentation which includes an update to
> the documentation for list_for_each_entry_continue_rcu() to match the
> documentation for the new list_for_each_entry_from_rcu(), and adds
> list_for_each_entry_from_rcu() and the already existing
> hlist_for_each_entry_from_rcu() to section 7 of whatisRCU.txt.
>
> Reviewed-by: Paul E. McKenney <[email protected]>
> Signed-off-by: NeilBrown <[email protected]>

Once I rebase my commits to v4.18-rc1, I will apply this, thank you!

(If you were wanting something else to happen with this patch, please
let me know.)

Thanx, Paul

> ---
> Documentation/RCU/whatisRCU.txt | 2 ++
> include/linux/rculist.h | 19 ++++++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
> index 65eb856526b7..d43524493fb3 100644
> --- a/Documentation/RCU/whatisRCU.txt
> +++ b/Documentation/RCU/whatisRCU.txt
> @@ -816,11 +816,13 @@ RCU list traversal:
> list_next_rcu
> list_for_each_entry_rcu
> list_for_each_entry_continue_rcu
> + list_for_each_entry_from_rcu
> hlist_first_rcu
> hlist_next_rcu
> hlist_pprev_rcu
> hlist_for_each_entry_rcu
> hlist_for_each_entry_rcu_bh
> + hlist_for_each_entry_from_rcu
> hlist_for_each_entry_continue_rcu
> hlist_for_each_entry_continue_rcu_bh
> hlist_nulls_first_rcu
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index 36df6ccbc874..4786c2235b98 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -396,7 +396,16 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> * @member: the name of the list_head within the struct.
> *
> * Continue to iterate over list of given type, continuing after
> - * the current position.
> + * the current position which must have been in the list when the RCU read
> + * lock was taken.
> + * This would typically require either that you obtained the node from a
> + * previous walk of the list in the same RCU read-side critical section, or
> + * that you held some sort of non-RCU reference (such as a reference count)
> + * to keep the node alive *and* in the list.
> + *
> + * This iterator is similar to list_for_each_entry_from_rcu() except
> + * this starts after the given position and that one starts at the given
> + * position.
> */
> #define list_for_each_entry_continue_rcu(pos, head, member) \
> for (pos = list_entry_rcu(pos->member.next, typeof(*pos), member); \
> @@ -411,6 +420,14 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> *
> * Iterate over the tail of a list starting from a given position,
> * which must have been in the list when the RCU read lock was taken.
> + * This would typically require either that you obtained the node from a
> + * previous walk of the list in the same RCU read-side critical section, or
> + * that you held some sort of non-RCU reference (such as a reference count)
> + * to keep the node alive *and* in the list.
> + *
> + * This iterator is similar to list_for_each_entry_continue_rcu() except
> + * this starts from the given position and that one starts from the position
> + * after the given position.
> */
> #define list_for_each_entry_from_rcu(pos, head, member) \
> for (; &(pos)->member != (head); \
> --
> 2.14.0.rc0.dirty
>



2018-06-20 15:32:16

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rculist: improve documentation for list_for_each_entry_from_rcu()

On Mon, Jun 18, 2018 at 10:01:11AM -0700, Paul E. McKenney wrote:
> On Mon, Jun 18, 2018 at 02:22:40PM +1000, NeilBrown wrote:
> >
> > Unfortunately the patch for adding list_for_each_entry_from_rcu()
> > wasn't the final patch after all review. It is functionally
> > correct but the documentation was incomplete.
> >
> > This patch adds this missing documentation which includes an update to
> > the documentation for list_for_each_entry_continue_rcu() to match the
> > documentation for the new list_for_each_entry_from_rcu(), and adds
> > list_for_each_entry_from_rcu() and the already existing
> > hlist_for_each_entry_from_rcu() to section 7 of whatisRCU.txt.
> >
> > Reviewed-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: NeilBrown <[email protected]>
>
> Once I rebase my commits to v4.18-rc1, I will apply this, thank you!
>
> (If you were wanting something else to happen with this patch, please
> let me know.)

And hearing no objections, I have applied it.

Thanx, Paul