lkml.org 
[lkml]   [2010]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/9] ACPI, APEI, Add ERST record ID cache
    On Wed, Oct 20, 2010 at 09:36:52AM +0800, Huang Ying wrote:
    > 1
    > 2
    > 3
    > 4
    > -1
    > -1
    >
    > where -1 signals there is no more record ID.
    >
    > Reader 1 has no chance to check record 2 and 4, while reader 2 has no
    > chance to check record 1 and 3. And any other GET_NEXT_RECORD_ID will
    > return -1, that is, other readers will has no chance to check any
    > record even they are not cleared by anyone.
    >
    > This makes raw GET_NEXT_RECORD_ID not suitable for usage of multiple
    > users.
    >
    > To solve the issue, an in memory ERST record ID cache is designed and
    > implemented. When enumerating record ID, the ID returned by
    > GET_NEXT_RECORD_ID is added into cache in addition to be returned to
    > caller. So other readers can check the cache to get all record ID
    > available.

    Generally it looks ok, just a minor cleanup nit below.

    Reviewed-by: Andi Kleen <ak@linux.intel.com>

    > +static int erst_record_id_cache_add_one(void)
    > +{
    > + u64 id, prev_id, first_id;
    > + int i, rc;
    > + struct erst_record_id_entry *entries;
    > + unsigned long flags;
    > +
    > + id = prev_id = first_id = APEI_ERST_INVALID_RECORD_ID;
    > +retry:
    > + spin_lock_irqsave(&erst_lock, flags);
    > + rc = __erst_get_next_record_id(&id);
    > + spin_unlock_irqrestore(&erst_lock, flags);
    > + if (rc == -ENOENT)
    > + return 0;
    > + if (rc)
    > + return rc;
    > + if (id == APEI_ERST_INVALID_RECORD_ID)
    > + return 0;
    > + /* can not skip current ID, or look back to first ID */
    > + if (id == prev_id || id == first_id)
    > + return 0;
    > + if (first_id == APEI_ERST_INVALID_RECORD_ID)
    > + first_id = id;
    > + prev_id = id;
    > +
    > + entries = erst_record_id_cache.entries;
    > + for (i = 0; i < erst_record_id_cache.len; i++) {
    > + if (!entries[i].cleared && entries[i].id == id)
    > + break;
    > + }
    > + /* record id already in cache, try next */
    > + if (i < erst_record_id_cache.len)
    > + goto retry;
    > + if (erst_record_id_cache.len >= erst_record_id_cache.size) {
    > + int new_size, alloc_size;
    > + struct erst_record_id_entry *new_entries;
    > +
    > + new_size = erst_record_id_cache.size * 2;
    > + new_size = max_t(int, new_size, ERST_RECORD_ID_CACHE_SIZE_MIN);
    > + new_size = min_t(int, new_size, ERST_RECORD_ID_CACHE_SIZE_MAX);

    This is clamp_t()

    > + if (new_size <= erst_record_id_cache.size) {
    > + if (printk_ratelimit())
    > + pr_warning(FW_WARN ERST_PFX
    > + "too many record ID!\n");
    > + return 0;
    > + }
    > + alloc_size = new_size * sizeof(struct erst_record_id_entry);
    > + if (alloc_size < PAGE_SIZE)
    > + new_entries = kmalloc(alloc_size, GFP_KERNEL);
    > + else
    > + new_entries = vmalloc(alloc_size);

    This is essentially kremalloc with vmalloc. Since this a common
    pattern it would be nicer to put a generic helper for this somewhere.

    -Andi

    > + if (!new_entries)
    > + return -ENOMEM;
    > + memcpy(new_entries, entries,
    > + erst_record_id_cache.len * sizeof(entries[0]));
    > + if (erst_record_id_cache.size < PAGE_SIZE)
    > + kfree(entries);
    > + else
    > + vfree(entries);

    -Andi
    --
    ak@linux.intel.com -- Speaking for myself only.


    \
     
     \ /
      Last update: 2010-10-22 14:07    [W:2.294 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site