From: Alex Dubov on
--- On Tue, 3/8/10, Maxim Levitsky <maximlevitsky(a)gmail.com> wrote:

> Now that del_gendisk syncs, we better
> start rejecting requests right away.


I don't quite see why this change is needed. My understanding is, user
accessible interface should be marked as removed as early, as possible.


>
> Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> ---
> drivers/memstick/core/mspro_block.c |� � 5
> +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memstick/core/mspro_block.c
> b/drivers/memstick/core/mspro_block.c
> index 8327e24..8d27c13 100644
> --- a/drivers/memstick/core/mspro_block.c
> +++ b/drivers/memstick/core/mspro_block.c
> @@ -1330,13 +1330,14 @@ static void
> mspro_block_remove(struct memstick_dev *card)
> ��� struct mspro_block_data *msb =
> memstick_get_drvdata(card);
> ��� unsigned long flags;
>
> -��� del_gendisk(msb->disk);
> -��� dev_dbg(&card->dev, "mspro block
> remove\n");
> ��� spin_lock_irqsave(&msb->q_lock,
> flags);
> ��� msb->eject = 1;
> ��� blk_start_queue(msb->queue);
> ���
> spin_unlock_irqrestore(&msb->q_lock, flags);
>
> +��� del_gendisk(msb->disk);
> +��� dev_dbg(&card->dev, "mspro block
> remove\n");
> +
> ��� blk_cleanup_queue(msb->queue);
> ��� msb->queue = NULL;
>
> --
> 1.7.0.4
>
>



--
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: Alex Dubov on
> From: Maxim Levitsky <maximlevitsky(a)gmail.com>
> Subject: Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk
> > >
> > > > Now that del_gendisk syncs, we better
> > > > start rejecting requests right away.
> > >
> > >
> > > I don't quite see why this change is needed. My
> understanding is, user
> > > accessible interface should be marked as removed
> as early, as possible.
> >
> > The problem here is that del_gendisk, syncs the
> device.
> > This is new change, made after you did your drivers.
> >
> > I have this problem on jMicron too (which otherwise
> works fine).
> The problem is that card check thread explicitly calls
> ->stop before
> removing the device.
> In case of mspro_blk.c that stops the request queue.
> Attempt to call del_gendisk with stopped request queue
> hangs due to
> syncing.

Well, ok. Sounds good.


> >
> > I have a copy of your ms_block.c.
> > I would would be very happy if you share with me, what
> problems does it
> > still have (besides need of trivial port for changes
> in block system,
> > because I want to push it upstream too.
> >
> > I have MS DUO 64M to test it against.
> >

I've got two rather different implementations of ms_block, if I remember
correctly. Both suffer from random data corruptions, thanks to my
inexplicable desire to write the state machine as tightly, as possible.
Only the later one does the spec mandated geometry correctly - I actually
wrote the first version before I've seen the spec for the first time.




--
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: Alex Dubov on

>
> By 'spec mandated geometry' you mean the fact the card
> needs to be
> broken into zones, somewhat like XD?
>

Yes, and in fancy fashion.

First zone should have at most 494 useful logical blocks, other zones - 496.
16 physical blocks per zone are considered local spares, 2 first blocks in
zone 1 must be identical and contain factory information.




--
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/