> > I'm worried about two things: > > > > 1) If the per-bdi threshold becomes smaller than the granularity of > > the per-bdi stat (due to the per-CPU counters), then things will > > break. Shouldn't there be some sanity checking for the calculated > > threshold? > > I'm not sure what you're referring to. > > void get_writeout_scale(struct backing_dev_info *bdi, int *scale, int *div) > { > int bits = vm_cycle_shift - 1; > unsigned long total = __global_bdi_stat(BDI_WRITEOUT_TOTAL); > unsigned long cycle = 1UL << bits; > unsigned long mask = cycle - 1; > > if (bdi_cap_writeback_dirty(bdi)) { > bdi_writeout_norm(bdi); > *scale = __bdi_stat(bdi, BDI_WRITEOUT); > } else > *scale = 0; > > *div = cycle + (total & mask); > } > > where cycle ~ vm_total_pages > scale can be a tad off due to overstep here: > > void __inc_bdi_stat(struct backing_dev_info *bdi, enum bdi_stat_item item) > { > struct bdi_per_cpu_data *pcd = &bdi->pcd[smp_processor_id()]; > s8 *p = pcd->bdi_stat_diff + item; > > (*p)++; > > if (unlikely(*p > pcd->stat_threshold)) { > int overstep = pcd->stat_threshold / 2; > > bdi_stat_add(*p + overstep, bdi, item); > *p = -overstep; > } > } > > so it could be that: scale / cycle > 1 > by a very small amount; however:
No, I'm worried about the case when scale is too small. If the per-bdi threshold becomes smaller than stat_threshold, then things won't work, because dirty+writeback will never go below the threshold, possibly resulting in the deadlock we are trying to avoid. BTW, the second argument of get_dirty_limits() doesn't seem to get used by the caller, or does it? > here we clip to 'reserve' which is the total amount of dirty threshold > not dirty by others. > > > 2) The loop is sleeping in congestion_wait(WRITE), which seems wrong. > > It may well be possible that none of the queues are congested, so > > it will sleep the full .1 second. But by that time the queue may > > have become idle and is just sitting there doing nothing. Maybe > > there should be a per-bdi waitq, that is woken up, when the per-bdi > > stats are updated. > > Good point, .1 seconds is a lot of time. > > I'll cook up something like that if nobody beats me to it :-) I realized, that it's maybe worth storing last the threshold in the bdi as well, so that balance_dirty_pages() doesn't get woken up too many times unnecessarilty. But I don't know... Thanks, Miklos - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/