Hi Ryusuke,
On Jan 2, 2014, at 8:17 PM, Ryusuke Konishi wrote:
> Hi, Andreas, and Vyacheslav
>
> Thank you for posting this patch.
>
> I reviewed the patch, and yes, the patch looks correct and needed to
> fix the retry logic of nilfs_segctor_collect().
>
> I am thinking of sending this to upstream. But, before that, please
> test the patch enough in the same situation to confirm that it
> actually fixes the problem and it doesn't cause other issues.
>
> At this point, I feel this patch has no defects nor side effects, but
> I cannot test it in the same situation until the next week.
>
if (sci->sc_stage.flags & NILFS_CF_SUFREED) {
err = nilfs_sufile_cancel_freev(nilfs->ns_sufile,
sci->sc_freesegs,
sci->sc_nfreesegs,
NULL);
WARN_ON(err); /* do not happen */
+ sci->sc_stage.flags &= ~NILFS_CF_SUFREED;
}
I see adding the code of clear NILFS_CF_SUFREED flag in the patch.
But nilfs_segctor_abort_construction() contains likewise code:
1727 if (sci->sc_stage.flags & NILFS_CF_SUFREED) {
1728 ret = nilfs_sufile_cancel_freev(nilfs->ns_sufile,
1729 sci->sc_freesegs,
1730 sci->sc_nfreesegs,
1731 NULL);
1732 WARN_ON(ret); /* do not happen */
1733 }
Maybe, this code needs in flag clearing too?
Anyway, I can see setting this flag and cannot see clearing
of it. Andreas suggests to clear this flag in the patch.
Is logic of setting/clearing this flag fully correct?
With the best regards,
Vyacheslav Dubeyko.
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
