Hi,
+/*
+ * get_super_without_lock - Get super_block from block_device without lock.
+ * @bdev: block device struct
+ *
+ * Scan the superblock list and finds the superblock of the file system
+ * mounted on the block device given. This doesn't lock anyone.
+ * %NULL is returned if no match is found.
+ */
This is not a terribly good comment.
Which lock are we not taking? I _assume_ that it's referring to
s_umount? If so, the text should describe that.
It should also go to some lengths explaining why this dangerous-looking
and rather nasty-looking function exists.
Look at it this way: there is no way in which the reviewer of this
patch (ie: me) can work out why this function exists. Hence there will
be no way in which future readers of this code will be able to work out
why this function exists either. This is bad. These things should be
described in code comments and in the changelog (whichever is most
appropriate).
Thank you for your comment. I will write comments appropriately.
I was wrong. I thought we didn't need to lock s_umount because
this ioctl required to open a regular file or a directory and we cannot
unmount a target filesystem.
So I created get_super_without_lock() used in freeze_bdev().
But, I have found that the ioctl (DM_DEV_SUSPEND_CMD in
drivers/md/dm-ioctl.c) requires to open a logical volume
(not a file or a directory) and calls freeze_bdev(), so we can unmount
a filesystem.
So I will replace get_super_without_lock with get_super to get s_umount.
Cheers, Takashi
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
