Message ID | 1466843991-3423-1-git-send-email-richard@nod.at |
---|---|
State | Superseded |
Headers | show |
Am 25.06.2016 um 10:39 schrieb Richard Weinberger: > Currently we read a whole (sub)page from flash when reading > the VID header. > Sascha reported that reading only UBI_VID_HDR_SIZE instead > gives him a decent speedup during a full scan. > > Since we have to obey shifted headers too we have to read > ubi->vid_hdr_shift + UBI_VID_HDR_SIZE bytes. > It is also important to set the buffer to 0xff bytes before > reading as in ubi_io_write_vid_hdr() the whole (sub)page > is written back to flash and we cannot risk leaking > kernel memory into the storage medium. > > Reported-by: Sascha Hauer <s.hauer@pengutronix.de> > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > Sascha, > > can you please this patch a try? > > Thanks, > //richard > --- > drivers/mtd/ubi/io.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c > index 10cf3b5..803a22d 100644 > --- a/drivers/mtd/ubi/io.c > +++ b/drivers/mtd/ubi/io.c > @@ -1018,8 +1018,9 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, > ubi_assert(pnum >= 0 && pnum < ubi->peb_count); > > p = (char *)vid_hdr - ubi->vid_hdr_shift; > + memset(p, 0xff, ubi->vid_hdr_alsize); Given a second thought on that, we can drop this memset(). > read_err = ubi_io_read(ubi, p, pnum, ubi->vid_hdr_aloffset, > - ubi->vid_hdr_alsize); > + ubi->vid_hdr_shift + UBI_VID_HDR_SIZE); > if (read_err && read_err != UBI_IO_BITFLIPS && !mtd_is_eccerr(read_err)) > return read_err; Thanks, //richard
On Sat, Jun 25, 2016 at 10:39:51AM +0200, Richard Weinberger wrote: > Currently we read a whole (sub)page from flash when reading > the VID header. > Sascha reported that reading only UBI_VID_HDR_SIZE instead > gives him a decent speedup during a full scan. > > Since we have to obey shifted headers too we have to read > ubi->vid_hdr_shift + UBI_VID_HDR_SIZE bytes. > It is also important to set the buffer to 0xff bytes before > reading as in ubi_io_write_vid_hdr() the whole (sub)page > is written back to flash and we cannot risk leaking > kernel memory into the storage medium. > > Reported-by: Sascha Hauer <s.hauer@pengutronix.de> > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > Sascha, > > can you please this patch a try? I already did, this was exactly my suggestion after you mentioned that my initial patch breaks when vid_hdr_shift is non zero. I tested it without the memset though. For reference: I tested this on a 8k page size Nand formatted with ubiformat -O 512 and attached with ubiattach -O 512. Sascha > > Thanks, > //richard > --- > drivers/mtd/ubi/io.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c > index 10cf3b5..803a22d 100644 > --- a/drivers/mtd/ubi/io.c > +++ b/drivers/mtd/ubi/io.c > @@ -1018,8 +1018,9 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, > ubi_assert(pnum >= 0 && pnum < ubi->peb_count); > > p = (char *)vid_hdr - ubi->vid_hdr_shift; > + memset(p, 0xff, ubi->vid_hdr_alsize); > read_err = ubi_io_read(ubi, p, pnum, ubi->vid_hdr_aloffset, > - ubi->vid_hdr_alsize); > + ubi->vid_hdr_shift + UBI_VID_HDR_SIZE); > if (read_err && read_err != UBI_IO_BITFLIPS && !mtd_is_eccerr(read_err)) > return read_err; > > -- > 2.7.3 > >
Am 27.06.2016 um 07:22 schrieb Sascha Hauer: > On Sat, Jun 25, 2016 at 10:39:51AM +0200, Richard Weinberger wrote: >> Currently we read a whole (sub)page from flash when reading >> the VID header. >> Sascha reported that reading only UBI_VID_HDR_SIZE instead >> gives him a decent speedup during a full scan. >> >> Since we have to obey shifted headers too we have to read >> ubi->vid_hdr_shift + UBI_VID_HDR_SIZE bytes. >> It is also important to set the buffer to 0xff bytes before >> reading as in ubi_io_write_vid_hdr() the whole (sub)page >> is written back to flash and we cannot risk leaking >> kernel memory into the storage medium. >> >> Reported-by: Sascha Hauer <s.hauer@pengutronix.de> >> Signed-off-by: Richard Weinberger <richard@nod.at> >> --- >> Sascha, >> >> can you please this patch a try? > > I already did, this was exactly my suggestion after you mentioned that > my initial patch breaks when vid_hdr_shift is non zero. > I tested it without the memset though. Yeah, the memset() is not needed, I was wrong. So we can take your version. Thanks, //richard
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 10cf3b5..803a22d 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -1018,8 +1018,9 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, ubi_assert(pnum >= 0 && pnum < ubi->peb_count); p = (char *)vid_hdr - ubi->vid_hdr_shift; + memset(p, 0xff, ubi->vid_hdr_alsize); read_err = ubi_io_read(ubi, p, pnum, ubi->vid_hdr_aloffset, - ubi->vid_hdr_alsize); + ubi->vid_hdr_shift + UBI_VID_HDR_SIZE); if (read_err && read_err != UBI_IO_BITFLIPS && !mtd_is_eccerr(read_err)) return read_err;
Currently we read a whole (sub)page from flash when reading the VID header. Sascha reported that reading only UBI_VID_HDR_SIZE instead gives him a decent speedup during a full scan. Since we have to obey shifted headers too we have to read ubi->vid_hdr_shift + UBI_VID_HDR_SIZE bytes. It is also important to set the buffer to 0xff bytes before reading as in ubi_io_write_vid_hdr() the whole (sub)page is written back to flash and we cannot risk leaking kernel memory into the storage medium. Reported-by: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Richard Weinberger <richard@nod.at> --- Sascha, can you please this patch a try? Thanks, //richard --- drivers/mtd/ubi/io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)