On 2012/09/14 11:36, Eric Sandeen wrote:
On 9/13/12 6:00 AM, Fernando Luis Vázquez Cao wrote:
iterate_supers() calls a function provided by the caller with the s_umount
semaphore taken in read mode. However, there may be cases where write mode
is preferable, so we add a new helper, __iterate_supers(), which lets one
specify the mode of the lock.
This will be used to fix the emergency thaw (filesystem unfreeze) code, which
iterates over the list of superblocks but needs to hold the s_umount semaphore
in _write_ mode bebore carrying out the actual thaw operation.
I'll go ahead & comment on this even though all of this is to support the
emergency thaw misfeature which I'm liking less and less this evening.
I think this split is a little odd, usually __foo() provides some fundamental
action, and foo_*() functions call it after possibly defining or altering aspects of
that action, or preparing for that action.
So having iterate_supers() do read locks, and __iterate_supers do either read
or write, seems asymmetric, and isn't very well self-documenting.
Why not have i.e. iterate_supers_read() and iterate_supers_write(), each
of which can call __iterate_supers(blah, blah, locktype).
Anyway, it'd mean changing 5 callers but it's a little clearer and more symmetric
to me. What do you think?
The logic seems fine, just a question about the style.
I agree with you. I will fix the style as you suggest.
Thanks,
Fernando
--
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
