On Mon, 17 Oct 2011, Joe Thornber wrote:
> On Fri, Oct 14, 2011 at 03:14:34PM -0400, Mikulas Patocka wrote:
> > -static struct dm_buffer *__bufio_new(struct dm_bufio_client *c, sector_t block, enum new_flag nf,
> > - struct dm_buffer **bp, int *need_submit)
> > +static void read_endio(struct bio *bio, int error);
> > +
> > +static struct dm_buffer *__bufio_new(struct dm_bufio_client *c, sector_t block,
> > + enum new_flag nf, struct dm_buffer **bp)
> > {
> > struct dm_buffer *b, *new_b = NULL;
> >
> > - *need_submit = 0;
> > b = __find(c, block);
> > if (b) {
> > b->hold_count++;
> > @@ -821,7 +837,9 @@ static struct dm_buffer *__bufio_new(str
> > }
> >
> > b->state = 1 << B_READING;
> > - *need_submit = 1;
> > +
> > + submit_io(b, READ, b->block, read_endio);
> > +
> > return b;
> > }
> >
> > @@ -849,22 +867,18 @@ static void read_endio(struct bio *bio,
> > static void *new_read(struct dm_bufio_client *c, sector_t block, enum new_flag nf,
> > struct dm_buffer **bp)
> > {
> > - int need_submit;
> > struct dm_buffer *b;
> >
> > dm_bufio_lock(c);
> > - b = __bufio_new(c, block, nf, bp, &need_submit);
> > + b = __bufio_new(c, block, nf, bp);
> > dm_bufio_unlock(c);
> >
> > if (!b || IS_ERR(b))
> > return b;
> > else {
> > - if (need_submit)
> > - submit_io(b, READ, b->block, read_endio);
> > -
> > wait_on_bit(&b->state, B_READING,
> > do_io_schedule, TASK_UNINTERRUPTIBLE);
>
>
> This change means submit_io(), which can block, will be called with
> the client lock held. I don't see this as an improvement. NACK.
dm-bufio is designed with the possibility that submit_io is called inside
the lock. There are other places that do it too. But it's true that it
wasn't called in the lock before.
I think the best thing would be to delete those functions and use the code
that was there before.
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
