Re: [PATCH 1/2] amiga floppy: Stop sharing request queue acrossmultiple gendisks

[Vivek Goyal]

On Thu, Oct 28, 2010 at 07:38:33PM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 23, 2010 at 21:54, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> > o Use one request queue per gendisk instead of sharing request queue
> >
> > o Don't have hardware. No compile testing or run time testing done. Completely
> > Âuntested.
> >
> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
> > ---
> > Âdrivers/block/amiflop.c | Â 59 ++++++++++++++++++++++++++++++++++++++--------
> > Â1 files changed, 48 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
> > index 76f114f..ead8b77 100644
> > --- a/drivers/block/amiflop.c
> > +++ b/drivers/block/amiflop.c
> > @@ -114,8 +114,6 @@ static unsigned long int fd_def_df0 = FD_DD_3; Â Â /* default for df0 if it does
> > Âmodule_param(fd_def_df0, ulong, 0);
> > ÂMODULE_LICENSE("GPL");
> >
> > -static struct request_queue *floppy_queue;
> > -
> > Â/*
> > Â* ÂMacros
> > Â*/
> > @@ -164,6 +162,7 @@ static volatile int selected = -1; Â/* currently selected drive */
> > Âstatic int writepending;
> > Âstatic int writefromint;
> > Âstatic char *raw_buf;
> > +static int fdc_queue;
> >
> > Âstatic DEFINE_SPINLOCK(amiflop_lock);
> >
> > @@ -1334,6 +1333,42 @@ static int get_track(int drive, int track)
> > Â Â Â Âreturn -1;
> > Â}
> >
> > +/*
> > + * Round-robin between our available drives, doing one request from each
> > + */
> > +static struct request *set_next_request(void)
> > +{
> > + Â Â Â struct request_queue *q;
> > + Â Â Â int cnt = FD_MAX_UNITS;
> > + Â Â Â struct request *rq;
> > +
> > + Â Â Â /* Find next queue we can dispatch from */
> > + Â Â Â fdc_queue = fdc_queue + 1;
> > + Â Â Â if (fdc_queue == FD_MAX_UNITS)
> > + Â Â Â Â Â Â Â fdc_queue = 0;
> > +
> > + Â Â Â for(cnt = FD_MAX_UNITS; cnt > 0, cnt--) {
> > +
> > + Â Â Â Â Â Â Â if (unit[fdc_queue].type->code == FD_NODRIVE) {
> > + Â Â Â Â Â Â Â Â Â Â Â if (++fdc_queue == FD_MAX_UNITS)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â fdc_queue = 0;
> > + Â Â Â Â Â Â Â Â Â Â Â cotinue;
> > + Â Â Â Â Â Â Â }
> > +
> > + Â Â Â Â Â Â Â q = unit[fdc_queue].gendisk->queue;
> > + Â Â Â Â Â Â Â if (q) {
> > + Â Â Â Â Â Â Â Â Â Â Â rq = blk_fetch_request(q);
> > + Â Â Â Â Â Â Â Â Â Â Â if (rq)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > + Â Â Â Â Â Â Â }
> > +
> > + Â Â Â Â Â Â Â if (++fdc_queue == FD_MAX_UNITS)
> > + Â Â Â Â Â Â Â Â Â Â Â fdc_queue = 0;
> > + Â Â Â }
> > +
> > + Â Â Â return rq;
> > +}
> 
> drivers/block/amiflop.c:1344: warning: ârqâ may be used uninitialized
> in this function
> drivers/block/ataflop.c:1402: warning: ârqâ may be used uninitialized
> in this function
> 
> Should `rq' just be initialized to NULL? I looked at
> floppy.c:set_next_request(), but it's
> completely different.
> 

I think we should explicitly initialize rq=NULL so that we don't return
a garbage value if we can't find any rq to dispatch.

I will send a patch to fix that. 

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