On 14/01/15 17:29, Denis V. Lunev wrote:
On 14/01/15 16:34, Roman Kagan wrote:
On Wed, Jan 14, 2015 at 04:08:50PM +0300, Denis V. Lunev wrote:
On 14/01/15 16:03, Roman Kagan wrote:
On Tue, Dec 30, 2014 at 01:07:10PM +0300, Denis V. Lunev wrote:
+static int cache_bat(BlockDriverState *bs, uint32_t idx, uint32_t
new_data_off)
+{
+ int ret, i, off, cache_off;
+ int64_t first_idx, last_idx;
+ BDRVParallelsState *s = bs->opaque;
+ uint32_t *cache = s->bat_cache;
+
+ off = bat_offset(idx);
+ cache_off = (off / s->bat_cache_size) * s->bat_cache_size;
+
+ if (s->bat_cache_off != -1 && s->bat_cache_off != cache_off) {
+ ret = write_bat_cache(bs);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ first_idx = idx - (off - cache_off) / sizeof(uint32_t);
+ last_idx = first_idx + s->bat_cache_size / sizeof(uint32_t);
+ if (first_idx < 0) {
+ memcpy(s->bat_cache, &s->ph, sizeof(s->ph));
+ first_idx = 0;
+ cache = s->bat_cache + sizeof(s->ph) / sizeof(uint32_t);
+ }
+
+ if (last_idx > s->bat_size) {
+ memset(cache + s->bat_size - first_idx, 0,
+ sizeof(uint32_t) * (last_idx - s->bat_size));
+ }
+
+ for (i = 0; i < last_idx - first_idx; i++) {
+ cache[i] = cpu_to_le32(s->bat[first_idx + i]);
+ }
You're re-populating the bat_cache on every bat update. Why?
Shouldn't this be done only with empty cache, i.e. under
if (s->bat_cache_off == -1)?
reasonable, but condition should be a bit different, namely
if (s->bat_cache_off != -1 && s->bat_cache_off != cache_off) {
No, this is the condition to flush the cache which you already do.
Then, either upon flushing it or on the first entry into cache_bat(),
the cache is empty which is indicated by ->bat_cache_off == -1, at which
point you need to populate it from ->bat.
you are wrong. BAT cache is a single page of the whole BAT
which can be more than several MBs. This page should be
repopulated when the off is changed.
ok, you are spoken about write_bat_cache which effectively marks
cache as invalid. Thus we are both spoken about the same
condition and we are on the same page.
