Thread (11 messages) 11 messages, 2 authors, 2015-05-12

Re: [PATCH 0/5] a caching layer for raid 5/6

From: Christoph Hellwig <hch@infradead.org>
Date: 2015-05-11 12:23:47

Hi Shaohua,

here are a couple of notes from reading through the code in a bit more
detail:


Error retries:
  - What is the reason for retry_bio_list?  If a driver returns an
    I/O error to the higher levels it already has retried and came
    to the conclusion this is a permanent error.

Flushes:
  - no need to allocate a task here
  - no real need to clone the bio either

Tasks:
  - the completion argument passed to r5l_queue_bio is always the same,
    the code would be a lot simpler by removing this abstraction.
  - that would also allow allocating the task embedded in the range
    and cut down on memory allocations
  - we're not really manipulating the bio payload, so shouldn't a
    _fast cone be fine here?
    In fact why do we clone the bio at all?
  - r5l_queue_task should probably be split into two helpers
    for data vs parity
  - r5l_queue_bio and r5c_copy_bio should probably use bvec iterators
  - r5l_run_task does very different things for data vs metadata,
    it's proably better it.

Allocations:
  - most metadata pages are allocated as highmem leading to
    constant kmap/kunmap.  Maybe just allocate them as GFP_KERNEL
    to simplify things?

Misc:
  - where does the ioctl in r5l_ioctl anѕ associated functions come
    from?  There are not ioctls handler here so the naming seems
    rather confusing.
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help