On Mon, Sep 10, 2007 at 01:02:07AM +0530, Bhagi rathi wrote:
> XFS_IOCORE_RT | XFS_DIFLAG_REALTIME can be set from an ioctl (xfs_setattr).
> A directIO without holding ILOCK
> in shared in mode can read a wrong value of di_flag for real time decision.
> As a result, we may pass in-correct device
> during directIO as the proposed xfs_find_bdev_for_inode doesn't hold any
> lock in reading the flags. It is not based
> on iocore flags as well.
>
> On a secondary note, XFS_IOCORE_RT was set without holding iolock which
> seems to an issue. I tend to leave
> xfs_bmapi to decide BMAPI_DEVICE to xfs_iomap.
The previous locking doesn't help anything - if the value changes during
the direct I/O process we are in trouble anyway. Fortunately we always
hold the iolock in shared mode over any direct I/O, so taking this lock
in ->setattr will fix this pre-existing issue.
> What is the reason why this has to be seperated?
Because it does not belong into xfs_iomap. While there is at least some
common code between BMAPI_READ, BMAPI_WRITE and BMAPI_ALLOCATE calls so
sharing that makes some sense it does not at all for the other two which
this patch separates out in preparation of bigger changes in this area.
|