Verma, Vishal L wrote:
> > > @@ -560,15 +551,12 @@ static ssize_t delete_store(struct device *dev,
> > > struct device_attribute *attr,
> > > if (!victim)
> > > return -ENXIO;
> > >
> > > - rc = down_write_killable(&dax_region_rwsem);
> > > - if (rc)
> > > - return rc;
> > > - rc = down_write_killable(&dax_dev_rwsem);
> > > - if (rc) {
> > > - up_write(&dax_region_rwsem);
> > > - return rc;
> > > - }
> > > + device_lock(dev);
> > > + device_lock(victim);
> > > dev_dax = to_dev_dax(victim);
> > > + rc = down_write_killable(&dax_dev_rwsem);
> >
> > This begs the question, why down_write_killable(), but not
> > device_lock_interruptible()?
>
> Do you mean change the device_lock()s to device_lock_interruptible() in
> addition to the taking the rwsem (i.e. not instead of the rwsem..)?
I mean convert the rwsem to drop _killable.
> I guess I just restored what was there previously - but the
> interruptible variant makes sense, I can make that change.
So the original code did device_lock(), then the rework added killable
rwsem (deleted device_lock()), and now the fixes add device_lock() back.
So now that there is a mix of killable/interruptible lock usage all the
locks should agree.
Since there really is no risk of these operations being long running
there is no driving need to make them killable/interruptible, so go with
the simple option.
