[PATCH][resend][v2] afs: Remove erroneous seq |= 1 in volume lookup loop

[Oleg Nesterov]

On 08/15, David Howells wrote:
>
> lirongqing <roy.qing.li at gmail.com> wrote:
>
> > From: Li RongQing <lirongqing at baidu.com>
> >
> > The `seq |= 1` operation in the volume lookup loop is incorrect because:
> > seq is already incremented at start, making it odd in next iteration
> > which triggers lock, but The `|= 1` operation causes seq to be even
> > and unintended lockless operation
> >
> > Remove this erroneous operation to maintain proper lock sequencing.
> >
> > Signed-off-by: Li RongQing <lirongqing at baidu.com>
>
> I think you're probably right.  I think possibly this should have Oleg's patch
> cited in the Fixes: line:
>
>     commit 4121b4337146b64560d1e46ebec77196d9287802
>     afs: fix the usage of read_seqbegin_or_lock() in afs_lookup_volume_rcu()

Cough ;) git show 4121b4337146b64560d1e46ebec77196d9287802:fs/afs/callback.c

	static struct afs_volume *afs_lookup_volume_rcu(struct afs_cell *cell,
							afs_volid_t vid)
	{
		struct afs_volume *volume = NULL;
		struct rb_node *p;
		int seq = 1;

		do {
			/* Unfortunately, rbtree walking doesn't give reliable results
			 * under just the RCU read lock, so we have to check for
			 * changes.
			 */
			seq++; /* 2 on the 1st/lockless path, otherwise odd */
			read_seqbegin_or_lock(&cell->volume_lock, &seq);

			p = rcu_dereference_raw(cell->volumes.rb_node);
			while (p) {
				volume = rb_entry(p, struct afs_volume, cell_node);

				if (volume->vid < vid)
					p = rcu_dereference_raw(p->rb_left);
				else if (volume->vid > vid)
					p = rcu_dereference_raw(p->rb_right);
				else
					break;
				volume = NULL;
			}

		} while (need_seqretry(&cell->volume_lock, seq));

		done_seqretry(&cell->volume_lock, seq);
		return volume;
	}

So I think the patch was correct.

> Also, I can't help but feel the changes that Oleg's patch made should probably
> be wrapped in a macro in linux/seqlock.h.

Yes, I think this API can be improved. Even the documentation is wrong.
Please see https://lore.kernel.org/all/20231024120808.GA15382@redhat.com/

I'll try to make another attempt next week...

Oleg.