Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
On Thu, Aug 25 2005, Jon Escombe wrote: > Alan Cox wrote: > >@@ -1661,6 +1671,9 @@ > >where = ELEVATOR_INSERT_FRONT; > >rq->flags |= REQ_PREEMPT; > >} > >+ if (action == ide_next) > >+ where = ELEVATOR_INSERT_FRONT; > >+ > >__elv_add_request(drive->queue, rq, where, 0); > >ide_do_request(hwgroup, IDE_NO_IRQ); > >spin_unlock_irqrestore(&ide_lock, flags); > > > >Also puzzles me- why is this needed ? > > I wanted the park command to get in at the head of the queue (behind the > currently executing request). > > Contrary to the comments for ide_do_drive_cmd(), ide_next didn't appear > to do anything to achieve this? At least from my initial testing before > I made this change - it could take a second or so for the park command > to be issued if the disk was busy That part seems to have been lost, apparently. The above patch is correct. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
Alan Cox wrote: @@ -1661,6 +1671,9 @@ where = ELEVATOR_INSERT_FRONT; rq->flags |= REQ_PREEMPT; } + if (action == ide_next) + where = ELEVATOR_INSERT_FRONT; + __elv_add_request(drive->queue, rq, where, 0); ide_do_request(hwgroup, IDE_NO_IRQ); spin_unlock_irqrestore(&ide_lock, flags); Also puzzles me- why is this needed ? I wanted the park command to get in at the head of the queue (behind the currently executing request). Contrary to the comments for ide_do_drive_cmd(), ide_next didn't appear to do anything to achieve this? At least from my initial testing before I made this change - it could take a second or so for the park command to be issued if the disk was busy Regards, Jon. __ Email via Mailtraq4Free from Enstar (www.mailtraqdirect.co.uk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
You need the kernel side timeout. Consider this case One page of memory holds the parking code A second page is swapped to disk and holds the resume code You park the disk You wakeup You got to page in the resume code So you really do want the kernel helping to avoid a deadlock @@ -1661,6 +1671,9 @@ where = ELEVATOR_INSERT_FRONT; rq->flags |= REQ_PREEMPT; } + if (action == ide_next) + where = ELEVATOR_INSERT_FRONT; + __elv_add_request(drive->queue, rq, where, 0); ide_do_request(hwgroup, IDE_NO_IRQ); spin_unlock_irqrestore(&ide_lock, flags); Also puzzles me- why is this needed ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
Jens Axboe wrote:
Ok, I'll give you some hints to get you started... What you really want
to do, is:
- Insert a park request at the front of the queue
- On completion callback on that request, freeze the block queue and
schedule it for unfreeze after a given time
Am attaching a first attempt at a patch - for comments only - please
don't apply to a production system. I've not delved into the IDE code
before, so I've just been following my nose... In other words - It
appears to work for me - but I may be doing something crazy ;)
Having said that, I tested with a utility that repeatedly froze/thawed
hundreds of times while really hammering the disk with file copies, and
nothing oopsed or failed to checksum afterwards...
To do:
Move the /proc interface to sysfs. At the moment it's just a simple
'echo -n 1 > /proc/ide/hda/freeze' to freeze, and 0 to thaw.
Address Jens concerns about our userspace code falling over and leaving
the machine hung. I favour retaining a binary on/off interface (rather
than specifying a timeout up front), but having the IDE code auto-thaw
on a timer.. That way we can just keep writing 1's to it while we're
checking the accelerometer and wanting to keep it frozen, and if we
should die then it'll wake up by itself after a second or so...
Same again for libata (for T43 owners).
Regards,
Jon.
__
Email via Mailtraq4Free from Enstar (www.mailtraqdirect.co.uk)diff -urN linux-2.6.13-rc6.original/drivers/ide/ide-io.c linux-2.6.13-rc6/drivers/ide/ide-io.c
--- linux-2.6.13-rc6.original/drivers/ide/ide-io.c 2005-06-17 20:48:29.0 +0100
+++ linux-2.6.13-rc6/drivers/ide/ide-io.c 2005-08-24 20:56:31.0 +0100
@@ -1181,6 +1181,16 @@
}
/*
+ * Don't accept a request when the queue is stopped
+ * (unless we are resuming from suspend)
+ */
+ if (test_bit(QUEUE_FLAG_STOPPED, &drive->queue->queue_flags) && !blk_pm_resume_request(rq)) {
+ printk(KERN_ERR "%s: queue is stopped!\n", drive->name);
+ hwgroup->busy = 0;
+ break;
+ }
+
+ /*
* Sanity: don't accept a request that isn't a PM request
* if we are currently power managed. This is very important as
* blk_stop_queue() doesn't prevent the elv_next_request()
@@ -1661,6 +1671,9 @@
where = ELEVATOR_INSERT_FRONT;
rq->flags |= REQ_PREEMPT;
}
+ if (action == ide_next)
+ where = ELEVATOR_INSERT_FRONT;
+
__elv_add_request(drive->queue, rq, where, 0);
ide_do_request(hwgroup, IDE_NO_IRQ);
spin_unlock_irqrestore(&ide_lock, flags);
diff -urN linux-2.6.13-rc6.original/drivers/ide/ide-proc.c linux-2.6.13-rc6/drivers/ide/ide-proc.c
--- linux-2.6.13-rc6.original/drivers/ide/ide-proc.c 2005-06-17 20:48:29.0 +0100
+++ linux-2.6.13-rc6/drivers/ide/ide-proc.c 2005-08-24 21:51:14.0 +0100
@@ -264,6 +264,122 @@
return -EINVAL;
}
+static int proc_ide_read_freeze
+ (char *page, char **start, off_t off, int count, int *eof, void *data)
+{
+ ide_drive_t *drive = (ide_drive_t *) data;
+ char *out = page;
+ int len;
+
+ proc_ide_settings_warn();
+
+ if (test_bit(QUEUE_FLAG_STOPPED, &drive->queue->queue_flags))
+ out += sprintf(out, "%s: queue is stopped\n", drive->name);
+ else
+ out += sprintf(out, "%s: queue not stopped\n", drive->name);
+
+ len = out - page;
+ PROC_IDE_READ_RETURN(page,start,off,count,eof,len);
+}
+
+void ide_end_freeze_rq(struct request *rq)
+{
+ struct completion *waiting = rq->waiting;
+ u8 *argbuf = rq->buffer;
+
+ /* Spinlock is already acquired */
+ if (argbuf[3] == 0xc4) {
+ blk_stop_queue(rq->q);
+ printk(KERN_ERR "ide_end_freeze_rq(): Queue stopped...\n");
+ }
+ else
+ printk(KERN_ERR "ide_end_freeze_rq(): Head not parked...\n");
+/*
+ blk_stop_queue(rq->q);
+ printk(KERN_ERR "ide_end_freeze_rq(): Queue stopped...\n");
+*/
+ complete(waiting);
+}
+
+static int proc_ide_write_freeze(struct file *file, const char __user *buffer,
+ unsigned long count, void *data)
+{
+ DECLARE_COMPLETION(wait);
+ unsigned long val, flags;
+ char *buf, *s;
+ struct request rq;
+ ide_drive_t *drive = (ide_drive_t *) data;
+ u8 args[7], *argbuf = args;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ proc_ide_settings_warn();
+
+ if (count >= PAGE_SIZE)
+ return -EINVAL;
+
+ s = buf = (char *)__get_free_page(GFP_USER);
+ if (!buf)
+ return -ENOMEM;
+
+ if (copy_from_user(buf, buffer, count)) {
+ free_page((unsigned long)buf);
+ return -EFAULT;
+ }
+
+ buf[count] = '\0';
+ memset(&rq, 0, sizeof(rq));
+ memset(&args, 0, sizeof(args));
+
+ /* Ought to check we're the right sort of device - i.e. hard disk only */
+
+ /* STANDY IMMEDIATE COMMAND (spins down drive - more obvious for testing?)
+ argbuf[0] = 0xe0;
+ */
+
+ /* UNLOAD IMMEDIATE COMMAND */
+ argbuf[0] = 0xe1;
+ argbuf[1] = 0x44;
+ argbuf[3] = 0x4c;
+ argbuf[4] = 0x4e;
+ argbuf[5] = 0x55;
+
+ /* Ought to have some sanity checking around these values */
+ val = simple_strtoul(buf, &s, 10);
+ if (val) {
+ /* Check we'
RE: [Hdaps-devel] Re: HDAPS, Need to park the head for real
> On Fri, Aug 19 2005, Jon Escombe wrote: > > For hard disk protection, I prefer the idea of the userspace code > > thawing the drive based on current accelerometer data, rather than > > simply waking up after x seconds (maybe you're running for > a bus rather > > than falling off a table)... > > > > To get the best of both worlds, could we maybe take a > watchdog timer > > approach, and have the timeout reset by the userspace component > > periodically re-requesting freeze? > > That would work, you can just define the semantics to be that echo > foo > frozen would add foo seconds to the timeout (or thaw > it, if foo is > 0). This one is really a hard one to ask for. I mean, if we can make it the way that it will keep knowing that the accel is changing heavily, then it would be great. This way we/users can implement other actions as well, not only for HDAPS, but the fact of kicking any other daemon that we want to. i.e. The theft system, kicking in laptop_mode if there is soft vibration for a certain amount of seconds, making festival tell you that the PC is being moved... Anything! The fact is also that if we would only make a driver for HDAPS, we could simply make it freeze for 8 seconds and done. How often do you drop the laptop? How long does it take even if it rolls down the stairs? 4 Seconds tops? But then, the driver would be boring. ;-) .Alejandro - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
Dave Hansen wrote: > In reality, that probably means a statically compiled daemon that > mlock()s itself, and any structures that it will need. It _might_ even > need to keep an open file descriptor on the "frozen" file. Because, in > theory, that file could be written out to the sysfs backing store. with such a hassle to make the parking API available, assure that the head parking daemon is not swapped out, can access the filedescriptor, has a priority high enough to start immediatly when needed, wouldn't that qualify for running in kernel space? Stefan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
On Fri, Aug 19 2005, Jon Escombe wrote: > > Please make it "echo 1 > frozen", then userspace can do "echo 0 > > frozen" > after five seconds. > > > >>>What if the code to do "echo 0 > frozen" is swapped out to disk? ;) > >>> > >>> > >>Emergency head parker needs to be pagelocked for other reasons. You do > >>not want to page it from disk while your notebook is in free fall. > >> > >> > > > >It's still a very bad idea imho, what if the head parker daemon is > >killed for other reasons? The automatic timeout thawing the drive is > >much saner. > > > > > For hard disk protection, I prefer the idea of the userspace code > thawing the drive based on current accelerometer data, rather than > simply waking up after x seconds (maybe you're running for a bus rather > than falling off a table)... > > To get the best of both worlds, could we maybe take a watchdog timer > approach, and have the timeout reset by the userspace component > periodically re-requesting freeze? That would work, you can just define the semantics to be that echo foo > frozen would add foo seconds to the timeout (or thaw it, if foo is 0). -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
Please make it "echo 1 > frozen", then userspace can do "echo 0 > frozen" after five seconds. What if the code to do "echo 0 > frozen" is swapped out to disk? ;) Emergency head parker needs to be pagelocked for other reasons. You do not want to page it from disk while your notebook is in free fall. It's still a very bad idea imho, what if the head parker daemon is killed for other reasons? The automatic timeout thawing the drive is much saner. For hard disk protection, I prefer the idea of the userspace code thawing the drive based on current accelerometer data, rather than simply waking up after x seconds (maybe you're running for a bus rather than falling off a table)... To get the best of both worlds, could we maybe take a watchdog timer approach, and have the timeout reset by the userspace component periodically re-requesting freeze? Regards, Jon. __ Email via Mailtraq4Free from Enstar (www.mailtraqdirect.co.uk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
On Thu, 2005-08-18 at 17:15 -0400, Adam Goode wrote: > On Thu, 2005-08-18 at 22:49 +0200, Pavel Machek wrote: > > Please make it "echo 1 > frozen", then userspace can do "echo 0 > frozen" > > after five seconds. > > What if the code to do "echo 0 > frozen" is swapped out to disk? ;) In the real world, to be really sure that you're not doing a trip out to the disk, you'll need a daemon which doesn't do any allocations between when it's notified and when it does the write to the control file. In reality, that probably means a statically compiled daemon that mlock()s itself, and any structures that it will need. It _might_ even need to keep an open file descriptor on the "frozen" file. Because, in theory, that file could be written out to the sysfs backing store. -- Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
On Tue, Aug 16 2005, Alejandro Bonilla Beeche wrote: If I were in your position, I would just implement this for ide (pata, not sata) right now, since that is what you need to support (or do some of these notebooks come with sata?). So it follows that you add an ide Some notebooks are coming up with a Sata controller I think, but is still and IDE drive. I think some T43's come with that. But, I will ask or check again later if we ever need this feature for SATA. I can confirm that T43's are using libata. I've tweaked the passthrough code to return the status registers (so we can tell whether the disk is parking sucessfully) http://groups.google.co.uk/group/fa.linux.kernel/browse_frm/thread/bd6b65dfcd1a3227 Regards, Jon. __ Email via Mailtraq4Free from Enstar (www.mailtraqdirect.co.uk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Hdaps-devel] Re: HDAPS, Need to park the head for real
On 8/16/05, Alejandro Bonilla Beeche <[EMAIL PROTECTED]> wrote: > On Tue, 2005-08-16 at 22:07 +0200, Jens Axboe wrote: > > On Tue, Aug 16 2005, Alejandro Bonilla Beeche wrote: > > If I were in your position, I would just implement this for ide (pata, > > not sata) right now, since that is what you need to support (or do some > > of these notebooks come with sata?). So it follows that you add an ide > > Some notebooks are coming up with a Sata controller I think, but is > still and IDE drive. I think some T43's come with that. > > But, I will ask or check again later if we ever need this feature for > SATA. I believe T43s use a SATA->PATA bridge for their hard drives, so we probably would. (see http://www.thinkwiki.org/wiki/Category_talk:T43). Yani - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
