Sorry about replying so late.
On Mon, 17 June 2013 21:10:53 +0800, vaughan wrote:
>
> Rewrite the last patch.
> Add a new field 'toopen' in sg_device to count ongoing sg_open's. By checking both 'toopen' and 'exclude' marks when do exclusive open, old race conditions can be avoided.
> Replace global sg_open_exclusive_lock with a per device lock - sfd_lock. Since sfds list is now protected by the lock owned by the same sg_device, sg_index_lock becomes a real global lock to only protect sg devices lookup.
> Also did some cleanup, such as remove get_exclude() and rename set_exclude() to clear_exclude().
>
...
> @@ -171,10 +168,10 @@ typedef struct sg_device { /* holds the state of each scsi generic device */
> wait_queue_head_t o_excl_wait; /* queue open() when O_EXCL in use */
> int sg_tablesize; /* adapter's max scatter-gather table size */
> u32 index; /* device index number */
> - /* sfds is protected by sg_index_lock */
> + spinlock_t sfd_lock; /* protect sfds, exclude, toopen */
> struct list_head sfds;
> + int toopen; /* number of who are ready to open sg */
^
I think the 'toopen' is a bad choice. I'm having trouble wrapping my
head around the semantics of this variable, your description feels a
bit handwavy, the main noun is missing in the command above, I think I
found one more overflow bug,...
What you ended up doing is reimplement a rw_semaphone. Why not use
one instead? down_write() for exclusive access, down_read() for
non-exclusive, _trylock variants for nonblocking opens, etc.
Would this work?
Jörn
--
I've never met a human being who would want to read 17,000 pages of
documentation, and if there was, I'd kill him to get him out of the
gene pool.
-- Joseph Costello
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
