On Tue 23-11-10 11:55:30, Jan Kara wrote: > On Tue 23-11-10 00:31:56, Dmitry wrote: > > On Mon, 22 Nov 2010 22:12:09 +0100, Jan Kara <jack@xxxxxxx> wrote: > > > On Thu 11-11-10 15:14:25, Dmitry Monakhov wrote: > > > > The only reader which use state_lock is dqget(), and is it serialized > > > > with quota_disable via SRCU. And state_lock doesn't guaranties anything > > > > for that case. All methods which modify quota flags already protected by > > > > dqonoff_mutex. Get rid of useless state_lock. > > > dq_state_lock has two properties you miss here: > > > a) It guarantees we read a consistent value from a quota state variable. > > So what, it is not guaranteed anything. Quota make becomes disabled > > one nanosecond after check. > > And with srcu_read_lock we can be guarantee what quota will be available > > for the caller. Even in case of stale value. > Well, that's not completely my point. The point is that when we > manipulate flags in an non-atomic way (and using &=, |=, ... is a > non-atomic way), we are not really guaranteed what values a reader will see > while the change happens. So in theory the reader could see that all quotas > are disabled during a transition from usrquota enabled to usrquota+grpquota > enabled (I admit a stupid example but you get the point). Looking into the code once more I should add that this has been a possible problem even before your patch. So it's not a problem you introduced. You just made it more visible ;). Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
