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
| ||
|
Message-ID: <CAFR8uedP7pGm+KbW4q0fR3L1GS6GwG0yAGUDBsJbrTfKNHnWag@mail.gmail.com> Date: Fri, 10 Aug 2012 08:29:45 -0700 From: Muthu Kumar <muthu.lkml@...il.com> To: Dave Chinner <david@...morbit.com> Cc: majianpeng <majianpeng@...il.com>, Neil Brown <neilb@...e.de>, axboe <axboe@...nel.dk>, "konrad.wilk" <konrad.wilk@...cle.com>, "chris.mason" <chris.mason@...ionio.com>, viro <viro@...iv.linux.org.uk>, tytso <tytso@....edu>, "adilger.kernel" <adilger.kernel@...ger.ca>, shaggy <shaggy@...nel.org>, mfasheh <mfasheh@...e.com>, jlbec <jlbec@...lplan.org>, bpm <bpm@....com>, elder <elder@...nel.org>, jfs-discussion <jfs-discussion@...ts.sourceforge.net>, linux-kernel <linux-kernel@...r.kernel.org>, xfs <xfs@....sgi.com>, linux-btrfs <linux-btrfs@...r.kernel.org>, linux-ext4 <linux-ext4@...r.kernel.org>, linux-raid <linux-raid@...r.kernel.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org> Subject: Re: Re: [PATCH 0/8] Set bi_rw when alloc bio before call bio_add_page. [ Resending in plain text... sorry for the duplicate ] Hi, On Mon, Jul 30, 2012 at 6:14 PM, Dave Chinner <david@...morbit.com> wrote: > > On Tue, Jul 31, 2012 at 08:55:59AM +0800, majianpeng wrote: > > On 2012-07-31 05:42 Dave Chinner <david@...morbit.com> Wrote: > > >On Mon, Jul 30, 2012 at 03:14:28PM +0800, majianpeng wrote: > > >> When exec bio_alloc, the bi_rw is zero.But after calling > > >> bio_add_page, > > >> it will use bi_rw. > > >> Fox example, in functiion __bio_add_page,it will call > > >> merge_bvec_fn(). > > >> The merge_bvec_fn of raid456 will use the bi_rw to judge the merge. > > >> >> if ((bvm->bi_rw & 1) == WRITE) > > >> >> return biovec->bv_len; /* always allow writes to be mergeable */ > > > > > >So if bio_add_page() requires bi_rw to be set, then shouldn't it be > > >set up for every caller? I noticed there are about 50 call sites for > > >bio_add_page(), and you've only touched about 10 of them. Indeed, I > > >notice that the RAID0/1 code uses bio_add_page, and as that can be > > >stacked on top of RAID456, it also needs to set bi_rw correctly. > > >As a result, your patch set is nowhere near complete, not does it > > >document that bio_add_page requires that bi_rw be set before calling > > >(which is the new API requirement, AFAICT). > > There are many place call bio_add_page and I send some of those. Because > > my abilty, so I only send > > some patchs which i understand clearly. > > Sure, but my point is that there is no point changing only a few and > ignoring the great majority of callers. Either fix them all, fix it > some other way (e.g. API change), or remove the code from the RAID5 > function that requires it. A while back, we tried to address this by changing the alloc functions to take rw argument and set it (as per Jens suggestion). I guess the patch did not make it in. Please check: https://lkml.org/lkml/2011/7/11/275 and the follow ups. If needed, I can dust up that patch and resend it. > <snip> > > It's entirely possible that when bi_rw was added to struct > bvec_merge_data, the person who added it was mistaken that bi_rw was > set at this point in time when in fact it never has been. Hence it's > presence and reliance on it would be a bug. > > That's what I'm asking - is this actually beneificial, or should it > simply be removed from struct bvec_merge_data? Data is needed to > answer that question.... There are cases where we found it really beneficial to know the rw field to decide if the can be really merged or not. Regards, Muthu > > Cheers, > > Dave. > -- > Dave Chinner > david@...morbit.com > -- > 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists