libata: implement ->set_capacity()
in [Kernel]
|
Prev: [PATCH 1/1] staging: hv: Fix error checking in channel.c
Next: ftrace syscalls, PowerPC: Fixes and PowerPC raw syscall tracepoint implementation
From: James Bottomley on 13 May 2010 13:20 On Thu, 2010-05-13 at 18:54 +0200, Tejun Heo wrote: > Hello, > > On 05/13/2010 06:38 PM, James Bottomley wrote: > > Instead of making this a block sysfs attribute, since HPA is SATA only, > > why not make it a libata attribute for the disk? > > > > That way on unlock, you can unlock the HPA and then trigger a partition > > rescan of the block device (BLKRRPART) ... this is an ioctl, so you need > > user context, but you have it if you do it from the sysfs write routine. > > This looks to be a lot simpler than threading it up through SCSI and > > block. > > This doesn't have anything to do with sysfs. It's called from block > partition scan code when it detects a partition extends beyond the end > of the device. No user intervention at all and the mechanism has been > there for quite some years and possibly predates sysfs. Am I being > really slow or are you looking at something else? OK, so maybe I'm misunderstanding what you're trying to do. I thought this set_capacity thing was trying to unlock the HPA and then cause block to see a new capacity ... the latter is what calling BLKRRPART from the sysfs write would do. If it's just to unlock the HPA, then forget the RRPART piece. So I think the bit I missed was you're trying to do this programatically from boot time partition read to detect if the user previously disabled the HPA and partitioned the device? In which case you still have user context, you can call BLKRRPART here too. James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jens Axboe on 13 May 2010 13:50 On Thu, May 13 2010, Tejun Heo wrote: > Hello, > > On 05/13/2010 06:06 PM, James Bottomley wrote: > > I'm not sure this is such a good interface ... it sounds very error > > prone for what is effectively a binary lock/unlock. > > Well, the original block interface was like that. It has been used as > binary switch tho. The requested capacity is always ~0ULL and return > value smaller than the current capacity is ignored. I'm all for > dropping the capacity parameter and the return value from > ->set_capacity() so that it just unlocks native capacity and directly > sets the new capacity. Jens? Is there a valid case for setting the capacity less than the unlocked capacity? I would think the unlock/lock bool api is saner. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 13 May 2010 14:30 Hello, On 05/13/2010 07:40 PM, Jens Axboe wrote: > Is there a valid case for setting the capacity less than the unlocked > capacity? I would think the unlock/lock bool api is saner. IDE currently is the only user (and probably has been that way the whole time), so it is a binary thing. I have no idea why the original interface was designed that way. Looks like it tried to be too generic. Anyways, for the task at hand, the following should be enough. void (*unlock_native_capacity)(void); This simple signalling is how the current interface is being used anyway. If nobody objects, I'll replace ->set_capacity() with the above. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 13 May 2010 14:50 Hello, James. On 05/13/2010 07:18 PM, James Bottomley wrote: > So I think the bit I missed was you're trying to do this programatically > from boot time partition read to detect if the user previously disabled > the HPA and partitioned the device? In which case you still have user > context, you can call BLKRRPART here too. Yeah, that's what I'm trying to do but the functionality is already in the block layer and has been used by IDE layer for a long time. We just forgot to implement it for libata. The only thing I did was fixing up corner case handling in block layer a bit and adding a callback to sd so that the block layer call can be passed down to libata. So, it doesn't have anything to do with sysfs attributes, block or libata. We sure can trigger the rescan by invoking BLKRRPART from inside the partition scan code but I'm afraid that would be more convoluted than the currently existing implementation of restarting the scan inside the scan function. The only thing is that the existing block method is somewhat overly generic without any good reason, so simplifying that would be a good idea. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ben Hutchings on 15 May 2010 09:30
On Thu, 2010-05-13 at 17:56 +0200, Tejun Heo wrote: > Hello, Jens, James, Jeff, > > This patchset implements ->set_capacity() in libata so that HPA can be > unlocked on demand. Thanks Tejun. > 0001-block-restart-partition-scan-after-resizing-a-device.patch > 0002-SCSI-implement-sd_set_capacity.patch > 0003-libata-use-the-enlarged-capacity-after-late-HPA-unlo.patch > 0004-libata-implement-on-demand-HPA-unlocking.patch > > 0001 makes partition scan code to restart after ->set_capacity(). > This makes sure that partitions which start beyond the HPA limit are > discovered. > > 0002 implements ->set_capacity() in sd. > > 0003 makes libata accept device capacity larger than the initial one. > > 0004 implements ->set_capacity() in libata which asks libata EH to > unlock HPA, waits and returns the new capacity. > > Ben Hutchings suggeseted implementing ->set_capacity() in libata and > also reported the bug in the current partition scan code where it > fails to discover partitions which start beyond the HPA limit. [...] And I've already successfully tested the combination of patches 2-4. Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. |