Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1343123 > unrolled thread

Re: [PATCH] jffs2: Don't add summary entry when MTD write fails

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] jffs2: Don't add summary entry when MTD write fails David Woodhouse <dwmw2@infradead.org> - 2016-02-25 12:40 +0100
    Re: [PATCH] jffs2: Don't add summary entry when MTD write fails Thomas.Betker@rohde-schwarz.com - 2016-02-25 18:50 +0100

#1343123 — Re: [PATCH] jffs2: Don't add summary entry when MTD write fails

[Multipart message — attachments visible in raw view] — view raw

On Wed, 2015-11-11 at 21:27 +0100, Thomas Betker wrote:
> 
>  int jffs2_flash_direct_writev(struct jffs2_sb_info *c, const struct kvec *vecs,
>                               unsigned long count, loff_t to, size_t *retlen)
>  {
> +       int ret;
> +
> +       ret = mtd_writev(c->mtd, vecs, count, to, retlen);
> +
>         if (!jffs2_is_writebuffered(c)) {
>                 if (jffs2_sum_active()) {
>                         int res;
> +
> +                       if (ret ||
> +                           *retlen != iov_length((struct iovec *) vecs, count))
> +                               return ret;
> +
>                         res = jffs2_sum_add_kvec(c, vecs, count, (uint32_t) to);
>                         if (res) {
>                                 return res;

OK... but perhaps we can dispense with the separate 'ret' and 'res'
variables and the rats nest of conditions, and do something like:

   int ret;

   ret = mtd_writev(…);

   if (!ret && *retlen == iov_length(…) &&
       !jffs2_is_writebuffered(c) && jffs2_sum_active()) 
          ret = jffs2_sum_add_kvec(…);
   
   return ret;

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[toc] | [next] | [standalone]

#1343364

Hello David:

> > +       int ret;
> > +
> > +       ret = mtd_writev(c->mtd, vecs, count, to, retlen);
> > +
> >         if (!jffs2_is_writebuffered(c)) {
> >                 if (jffs2_sum_active()) {
> >                         int res;
> > +
> > +                       if (ret ||
> > +                           *retlen != iov_length((struct iovec *)
> vecs, count))
> > +                               return ret;
> > +
> >                         res = jffs2_sum_add_kvec(c, vecs, count, 
> (uint32_t) to);
> >                         if (res) {
> >                                 return res;
> 
> OK... but perhaps we can dispense with the separate 'ret' and 'res'
> variables and the rats nest of conditions, and do something like:
> 
>    int ret;
> 
>    ret = mtd_writev(…);
> 
>    if (!ret && *retlen == iov_length(…) &&
>        !jffs2_is_writebuffered(c) && jffs2_sum_active()) 
>           ret = jffs2_sum_add_kvec(…);
>    
>    return ret;

While the logic is the same, will the compiler generate the same code? 
When CONFIG_JFFS2_SUMMARY is not set, "if (jffs2_sum_active())" means "if 
(0)", and I would assume that the compiler removes the whole clause, "if" 
and all. However, I am not sure what happens with "if (!ret && whatever && 
0)".

That's why I was taking pains to keep the original control flow intact, 
even if it's a rat's nest (it is). If I remember correctly, 
jffs2_flash_direct_writev() is called quite often, and I didn't want 
performance to suffer. I may be completely wrong here, of course, but then 
why wasn't the original source code "if (!jffs2_is_writebuffered(c) && 
jffs2_sum_active())"?

Best regards,
Thomas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web