Prev: driver for mcs7830 (aka DeLOCK) USB ethernet adapter
Next: [PATCH] [MTD] CHIPS: Support for SST 49LF040B flash chip
From: Eric Sandeen on 12 Oct 2006 18:00 Badari Pulavarty wrote: > This is exactly the solution I proposed earlier (to check > buffer_mapped() before calling submit_bh()). > But at that time, Jan pointed out that the whole handling is wrong. > > But if this is the only case we need to handle, I am okay with this band > aid :) Doh! And we come full circle... ok let me go reread that thread, it got long enough I had to swap out... :) -Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Badari Pulavarty on 12 Oct 2006 18:00 Eric Sandeen wrote: > Andrew Morton wrote: > > >> On Thu, 12 Oct 2006 14:28:20 +0200 >> Jan Kara <jack(a)suse.cz> wrote: >> >> >> >>> Where can we call >>> journal_dirty_data() without PageLock? >>> >>> >> block_write_full_page() will unlock the page, so ext3_writepage() >> will run journal_dirty_data_fn() against an unlocked page. >> >> I haven't looked into the exact details of the race, but it should >> be addressable via jbd_lock_bh_state() or j_list_lock coverage >> > I'm testing with something like this now; seem sane? > > journal_dirty_data & journal_unmap_data both check do > jbd_lock_bh_state(bh) close to the top... journal_dirty_data_fn has checked > buffer_mapped before getting into journal_dirty_data, but that state may > change before the lock is grabbed. Similarly re-check after we drop the lock. > > This is exactly the solution I proposed earlier (to check buffer_mapped() before calling submit_bh()). But at that time, Jan pointed out that the whole handling is wrong. But if this is the only case we need to handle, I am okay with this band aid :) Thanks, Badari - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Badari Pulavarty on 12 Oct 2006 18:40 Eric Sandeen wrote: > Badari Pulavarty wrote: > > >> This is exactly the solution I proposed earlier (to check >> buffer_mapped() before calling submit_bh()). >> But at that time, Jan pointed out that the whole handling is wrong. >> >> But if this is the only case we need to handle, I am okay with this band >> aid :) >> > > Doh! > > And we come full circle... ok let me go reread that thread, it got long > enough I had to swap out... :) Don't bother. Lets see if this is the only case that needs fixing and move forward.. Thanks, Badari - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jan Kara on 13 Oct 2006 04:00 > Eric Sandeen wrote: > >Andrew Morton wrote: > > > > > >>On Thu, 12 Oct 2006 14:28:20 +0200 > >>Jan Kara <jack(a)suse.cz> wrote: > >> > >> > >> > >>>Where can we call > >>>journal_dirty_data() without PageLock? > >>> > >>> > >>block_write_full_page() will unlock the page, so ext3_writepage() > >>will run journal_dirty_data_fn() against an unlocked page. > >> > >>I haven't looked into the exact details of the race, but it should > >>be addressable via jbd_lock_bh_state() or j_list_lock coverage > >> > >I'm testing with something like this now; seem sane? > > > >journal_dirty_data & journal_unmap_data both check do > >jbd_lock_bh_state(bh) close to the top... journal_dirty_data_fn has > >checked buffer_mapped before getting into journal_dirty_data, but that > >state may > >change before the lock is grabbed. Similarly re-check after we drop the > >lock. > > > > > This is exactly the solution I proposed earlier (to check > buffer_mapped() before calling submit_bh()). > But at that time, Jan pointed out that the whole handling is wrong. Yes, and it was. However it turned out that there are more problems than I thought ;). > But if this is the only case we need to handle, I am okay with this band > aid :) I think Eric's patch may be a part of it. But we still need to check whether the buffer is not after EOF before submitting it (or better said just after we manage to lock the buffer). Because while we are waiting for the buffer lock, journal_unmap_buffer() can still come and steal the buffer - at least the write-out in journal_dirty_data() definitely needs the check if I haven't overlooked something. Honza -- Jan Kara <jack(a)suse.cz> SuSE CR Labs - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Eric Sandeen on 13 Oct 2006 12:10
Jan Kara wrote: >> This is exactly the solution I proposed earlier (to check >> buffer_mapped() before calling submit_bh()). >> But at that time, Jan pointed out that the whole handling is wrong. > Yes, and it was. However it turned out that there are more problems > than I thought ;). > >> But if this is the only case we need to handle, I am okay with this band >> aid :) > I think Eric's patch may be a part of it. But we still need to check whether > the buffer is not after EOF before submitting it (or better said just > after we manage to lock the buffer). Because while we are waiting for > the buffer lock, journal_unmap_buffer() can still come and steal the > buffer - at least the write-out in journal_dirty_data() definitely needs > the check if I haven't overlooked something. Ok, let me think on that today. My first reaction is that if we have the bh state lock and pay attention to mapped in journal_dirty_data(), then any blocks past EOF which have gotten unmapped by journal_unmap_buffer will be recognized as such (because they are now unmapped... without needing to check for past EOF...) and we'll be fine. As a datapoint, davej's stresstest (several fsx's and fsstresses) survived an overnight run on his box, which used to panic in < 2 hrs. Survived about 6 hours on my box until I intentionally stopped it; my box had added a write/truncate test in a loop, with a bunch of periodic syncs as well.... -Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |