On Wed, 25 Nov 2015, Mike Snitzer wrote:
> On Wed, Nov 25 2015 at 4:32pm -0500,
> Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
>
> > On Wed, Nov 25 2015 at 4:03pm -0500,
> > Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> >
> > > Device mapper used the field bi_private to point to dm_target_io. However,
> > > since kernel 3.15, the bi_private field is unused, and so the targets do
> > > not need to save and restore this field.
> > >
> > > This patch removes code that saves and restores bi_private from dm-cache,
> > > dm-snapshot and dm-verity.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> > >
> > > ---
> > > drivers/md/dm-cache-target.c | 3 ---
> > > drivers/md/dm-snap.c | 6 +-----
> > > drivers/md/dm-verity.c | 3 ---
> > > 3 files changed, 1 insertion(+), 11 deletions(-)
> > >
> > > Index: linux-4.4-rc2/drivers/md/dm-cache-target.c
> > > ===================================================================
> > > --- linux-4.4-rc2.orig/drivers/md/dm-cache-target.c 2015-11-24 15:33:56.000000000 +0100
> > > +++ linux-4.4-rc2/drivers/md/dm-cache-target.c 2015-11-24 15:34:35.000000000 +0100
> > > @@ -118,14 +118,12 @@ static void iot_io_end(struct io_tracker
> > > */
> > > struct dm_hook_info {
> > > bio_end_io_t *bi_end_io;
> > > - void *bi_private;
> > > };
> > >
> > > static void dm_hook_bio(struct dm_hook_info *h, struct bio *bio,
> > > bio_end_io_t *bi_end_io, void *bi_private)
> > > {
> > > h->bi_end_io = bio->bi_end_io;
> > > - h->bi_private = bio->bi_private;
> > >
> > > bio->bi_end_io = bi_end_io;
> > > bio->bi_private = bi_private;
> >
> > As you can see dm_hook_bio() goes on to modify bi_private.
> > dm-cache uses it to get the migration object associated with a bio for
> > the overwrite_endio() case.
> >
> > So NAK on the dm-cache change.
> >
> > How closely have you reviewed your change for snapshot and verity?
>
> Hmm, they follow the same pattern (hooking bi_private for use in bio
> endio). So you're saying we no longer need to preserve any upper layer
> (e.g. DM core) use of bi_private?
Yes - because dm core doesn't use it.
> I'm really not seeing the harm in
> doing so... but I also don't have a problem with reinstating such
> backup/restore code if/when the need arises.
>
> So it seems I've talked myself into your patch ;)
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
