linux-kernel - Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll interval

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120725024737.GA2395@aarontestpc.amd.com>
Date:	Wed, 25 Jul 2012 10:47:38 +0800
From:	Aaron Lu <aaron.lu@....com>
To:	Betty Dall <betty.dall@...com>
CC:	Jeff Garzik <jgarzik@...ox.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Lin Ming <minggr@...il.com>, <linux-kernel@...r.kernel.org>,
	<linux-pm@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
	<linux-ide@...r.kernel.org>, Aaron Lu <aaron.lwe@...il.com>
Subject: Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll
 interval

Hi Betty,

On Tue, Jul 24, 2012 at 12:55:06PM -0600, Betty Dall wrote:
> > > > diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> > > > index 2f159aa..78c4226 100644
> > > > --- a/drivers/scsi/sr.c
> > > > +++ b/drivers/scsi/sr.c
> > > > @@ -869,6 +869,7 @@ static int sr_probe(struct device *dev)
> > > >  	dev_set_drvdata(dev, cd);
> > > >  	disk->flags |= GENHD_FL_REMOVABLE;
> > > >  	add_disk(disk);
> > > > +	disk_events_set_poll_msecs(disk, 2000);
> > > 
> > > Could you check that disk event's poll_msecs is the default (-1) before
> > > setting it to 2s? I am thinking of a case when the probe happens after
> > > the call to disk_events_poll_msecs_store() and this code would overwrite
> > > the user specified value.
> > 
> > The block device sr0 is created by this driver in this probe function,
> > so the user should not be able to set the poll interval before probe,
> > right?
> 
> The add_disk() call happens immediately before the new
> disk_events_set_poll_msecs() call. add_disk() is what eventually creates
> the sysfs files

Right, and it's disk_add_events in add_disk that adds these sysfs files.

> and calls your new disk_events_set_poll_msecs().

No... there is no call to disk_events_set_poll_msecs in add_disk, when
the events for the disk is created by disk_alloc_events, the poll_msecs
of the event is initialized to the default value -1. And then
disk_add_events will create the sysfs files and add_disk will return,
and I'll change the default value of -1 to 2000 with the new function
I've made.

> It makes more sense to me to have the new call to
> disk_events_set_poll_msecs(disk, 2000) before the call to add_disk().

This is too early, since the events of the disk is not allocated yet.

I hope I've explained this clearly, if you see a problem, please let me
know, thanks.

-Aaron


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/