JFFS2 oops when writing to two partitions simultaneously

Joakim Tjernlund joakim.tjernlund at transmode.se
Thu Jan 26 10:09:24 EST 2012


>
> Orjan Friberg <of at flatfrog.com> wrote on 2012/01/26 15:07:07:
> >
> > On 01/26/2012 02:16 PM, Joakim Tjernlund wrote:
> > > Anyhow, I think it is stupid (and probably buggy) to have kfree and kmalloc
> > > as separate. Why is it not done at the same time?
> >
> > To me it looks like the lock must be held the entire time.  We can't
> > allow two contexts using (i.e. freeing/allocating/writing to) the
> > compressor's compr_buf.
> >
> > Maybe the lock used here should be on a per-compressor basis, rather
> > than on the list as a whole.  (The list lock is still needed when adding
> > to/removing from the list of course.)
>
> Did look some more and to me it seems like the whole kmalloc/kfree dance is pointless.
> The end result is the same as for JFFS2_COMPR_MODE_PRIORITY w.r.t buffer mgmt as the
> compressor doesn't own the buffer anyway. It gets freed by jffs2_free_comprbuf()
> after it has been written to flash.
>
> Maybe I am missing something?

Yes, I did. This dance is there to save you an extra kmalloc in most cases.

Maybe you can get away with removing the extra lock dropping around kfree/kmalloc
and if you do, you can simply the code to something like:

			if ((this->compr_buf_size < orig_slen) || !this->compr_buf) {
				tmp_buf = kmalloc(orig_slen, GFP_ATOMIC);
				if (!tmp_buf) {
					printk(KERN_WARNING "JFFS2: No memory for compressor allocation. (%d bytes)\n", orig_slen);
					continue;
				}
				kfree(this->compr_buf);
				this->compr_buf = tmp_buf;
				this->compr_buf_size = orig_slen;
			}

Then have a good look at whether to use orig_slen or orig_dlen

 Jocke




More information about the linux-mtd mailing list