On 10/09/2015 10:37 AM, Hillf Danton wrote: >>>> @@ -109,8 +109,8 @@ static void ext4_finish_bio(struct bio *bio) >>>> if (bio->bi_error) >>>> buffer_io_error(bh); >>>> } while ((bh = bh->b_this_page) != head); >>>> - bit_spin_unlock(BH_Uptodate_Lock, &head->b_state); >>>> local_irq_restore(flags); >>> >>> What if it takes 100ms to unlock after IRQ restored? >> >> I'm not sure I understand in what direction you are going? Care to >> elaborate? >> > Your change introduces extra time cost the lock waiter has to pay in > the case that irq happens before the lock is released. [CC filesystem and mm people. For reference the thread starts here: http://thread.gmane.org/gmane.linux.kernel/2056996 ] Right, I see what you mean and it's a good point but when doing the patches I was striving for correctness and starting a discussion, hence the RFC. In any case I'd personally choose correctness over performance always ;). As I'm not an fs/ext4 expert and have added the relevant parties (please use reply-all from now on so that the thread is not being cut in the middle) who will be able to say whether it impact is going to be that big. I guess in this particular code path worrying about this is prudent as writeback sounds like a heavily used path. Maybe the problem should be approached from a different angle e.g. drain_all_pages and its reliance on the fact that the IPI will always be delivered in some finite amount of time? But what if a cpu with disabled interrupts is waiting on the task issuing the IPI? > >>>> + bit_spin_unlock(BH_Uptodate_Lock, &head->b_state); >>>> if (!under_io) { >>>> #ifdef CONFIG_EXT4_FS_ENCRYPTION >>>> if (ctx) >>>> -- >>>> 2.5.0 >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
