Re: [PATCH] nvme: fix SRCU protection of nvme_ns_head list

From: Sagi Grimberg <sagi@grimberg.me>
To: Christoph Hellwig <hch@lst.de>
Cc: Caleb Sander <csander@purestorage.com>,
	Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
	linux-nvme@lists.infradead.org,
	Uday Shankar <ushankar@purestorage.com>,
	"Paul E. McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH] nvme: fix SRCU protection of nvme_ns_head list
Date: Mon, 21 Nov 2022 11:43:33 +0200	[thread overview]
Message-ID: <a7092b30-39db-a58e-ea65-2cfd12201426@grimberg.me> (raw)
In-Reply-To: <20221121074039.GA24507@lst.de>

>>>    	sector_t capacity = get_capacity(head->disk);
>>>    	int node;
>>> +	int srcu_idx;
>>>    +	srcu_idx = srcu_read_lock(&head->srcu);
>>>    	list_for_each_entry_rcu(ns, &head->list, siblings) {
>>>    		if (capacity != get_capacity(ns->disk))
>>>    			clear_bit(NVME_NS_READY, &ns->flags);
>>>    	}
>>> +	srcu_read_unlock(&head->srcu, srcu_idx);
>>
>> I don't think you need srcu here, rcu_read_lock/unlock is sufficient.
> 
> So the code obviously does not sleep.  But I wonder if anything speaks
> against mixing SRCU and RCU protection for the same list?

I am not an expert, but the point of (s)rcu_synchronize to fence the
reader critical section isn't it? so if the reader doesn't sleep, the
existing rcu_synchronize should be sufficient.