Re: [PATCH] kill BMAPI_DEVICE

["Bhagi rathi" <jahnu77@xxxxxxxxx>]

On 9/10/07, Christoph Hellwig <hch@xxxxxx> wrote:
>
> 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.


Yes.  I believe the proposed change can succeed after the fix you mentioned.

> 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.


What is the goal of the bigger changes planned?

-Saradhi.


[[HTML alternate version deleted]]