On 10/10/2011 01:00 PM, Jonathan Corbet wrote:
On Fri, 7 Oct 2011 00:11:05 -0700 Allison Henderson<achender@xxxxxxxxxxxxxxxxxx> wrote:+ /* + * If the journal block was not found in the list, + * add a new pair to the list + */ + if (!b_pair) { + b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS); + if (!b_pair) { + spin_unlock(&journal->j_pair_lock); + return -ENOMEM; + }Here too... that really needs to be GFP_ATOMIC. I'm wondering, though...it looks like, over a short period of time, you will create an unordered linked list containing one entry for every physical block in the journal. You'll take a lock and search the whole list for every block that is committed. Wouldn't it be better just to have an array indexed by the journal logical block offset? Less memory, faster access...? Or am I missing something fundamental here? Thanks, jon
Hi Jon, I think that's a good suggestion. Initially I had made it a linked list since most of the journals existing lists were implemented as linked lists, but since this list ends up being a fixed size, an array is a good optimization here. Thx! Allison -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
