Re: [RFC 2/1] seqlock: make the read_seqbegin_or_lock() API more simple and less error-prone ?

[Oleg Nesterov <oleg@redhat.com>]

On 09/30, David Howells wrote:
>
> It would also be nice to fix the static lock-balance detection stuff that you
> get when you enable advanced checking during a kernel build.  It doesn't
> seem to understand seqlocks.
>
> > 	-	nextseq = 0;
> > 	+	seq = 0;
>
> Perhaps an init function or macro that hides this bit?
>
> 	void init_read_seqlock(int *seq)
> 	{
> 		*seq = 0;
> 	}
>
> 	init_read_seqlock(&seq);
>
> or:
>
> 	#define INIT_READ_SEQBEGIN 0
>
> 	seq = INIT_READ_SEQBEGIN;

Agreed,

> Though if we can fold the whole loop inside a macro, that might make it easier
> to use.

Yes...

> d_walk() in fs/dcache.c might give you issues, though.

Yes. But note that it can use need_seqretry_xxx() too, see the patch below.

So. Do you think it makes any sense to add the new helper for the start?
Can you suggest a better name? Say, check_restart() ?

Oleg.

--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1355,7 +1355,7 @@ static void d_walk(struct dentry *parent, void *data,
 		spin_lock(&this_parent->d_lock);
 
 		/* might go back up the wrong parent if we have had a rename. */
-		if (need_seqretry(&rename_lock, seq))
+		if (need_seqretry_xxx(&rename_lock, &seq))
 			goto rename_retry;
 		/* go into the first sibling still alive */
 		hlist_for_each_entry_continue(dentry, d_sib) {
@@ -1366,7 +1366,7 @@ static void d_walk(struct dentry *parent, void *data,
 		}
 		goto ascend;
 	}
-	if (need_seqretry(&rename_lock, seq))
+	if (need_seqretry_xxx(&rename_lock, &seq))
 		goto rename_retry;
 	rcu_read_unlock();
 
@@ -1378,11 +1378,8 @@ static void d_walk(struct dentry *parent, void *data,
 rename_retry:
 	spin_unlock(&this_parent->d_lock);
 	rcu_read_unlock();
-	BUG_ON(seq & 1);
-	if (!retry)
-		return;
-	seq = 1;
-	goto again;
+	if (retry)
+		goto again;
 }
 
 struct check_mount {