Re: [PATCH v2] Net: ceph: messenger: Use local variable cursor in read_partial_msg_data()

On 10/18/2015 09:49 PM, Shraddha Barke wrote: > Use local variable cursor in place of >cursor in > read_partial_msg_data() > > Signed-off-by: Shraddha Barke This is a pretty minor comment, but the "Net" in your subject line is probably better *not* capitalized.

Fwd: Re: [PATCH] rbd: set max_sectors explicitly

Forgot to "Reply-all". -Alex Forwarded Message Subject: Re: [PATCH] rbd: set max_sectors explicitly Date: Fri, 16 Oct 2015 07:22:55 -0500 From: Alex Elder <el...@ieee.org> To: Ilya Dryomov <idryo...@gmail.com> On 10/07/2015 12:00 PM, Ilya Dryomov wrote:

Re: [PATCH] rbd: use writefull op for object size writes

dryo...@gmail.com> Looks good to me. Reviewed-by: Alex Elder <el...@linaro.org> > --- > drivers/block/rbd.c | 9 +++-- > net/ceph/osd_client.c | 13 + > 2 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/

Re: [PATCH] rbd: don't leak parent_spec in rbd_dev_probe_parent()

On 10/16/2015 04:50 AM, Ilya Dryomov wrote: > On Thu, Oct 15, 2015 at 11:10 PM, Alex Elder <el...@ieee.org> wrote: >> On 10/11/2015 01:03 PM, Ilya Dryomov wrote: >>> Currently we leak parent_spec and trigger a "parent reference >>> underflow" warning

Re: [PATCH] rbd: don't leak parent_spec in rbd_dev_probe_parent()

On 10/15/2015 01:31 PM, Ilya Dryomov wrote: > On Thu, Oct 15, 2015 at 7:10 PM, Alex Elder <el...@ieee.org> wrote: >> On 10/11/2015 01:03 PM, Ilya Dryomov wrote: >>> Currently we leak parent_spec and trigger a "parent reference >>> underflow" warning

Re: [PATCH] rbd: don't leak parent_spec in rbd_dev_probe_parent()

On 10/11/2015 01:03 PM, Ilya Dryomov wrote: > Currently we leak parent_spec and trigger a "parent reference > underflow" warning if rbd_dev_create() in rbd_dev_probe_parent() fails. > The problem is we take the !parent out_err branch and that only drops > refcounts; parent_spec that would've been

Re: [PATCH] rbd: don't leak parent_spec in rbd_dev_probe_parent()

On 10/15/2015 01:31 PM, Ilya Dryomov wrote: > On Thu, Oct 15, 2015 at 7:10 PM, Alex Elder <el...@ieee.org> wrote: >> On 10/11/2015 01:03 PM, Ilya Dryomov wrote: >>> Currently we leak parent_spec and trigger a "parent reference >>> underflow" warning

Re: [PATCH] libceph: check data_len in ->alloc_msg()

UG_ON in ceph_msg_data_cursor_init() ensures we don't > corrupt random memory should a buggy ->alloc_msg() return an unfit > ceph_msg. So this will catch the problem, just a little later. Reviewed-by: Alex Elder <el...@linaro.org> > While at it, I changed the "unknown tid&

Re: [PATCH] rbd: plug rbd_dev->header.object_prefix memory leak

o encapsulate some of this, something like rbd_dev_header_info_release(). As things stand, freeing of the object prefix appears sort of random. It wasn't easy to follow the life cycle of that field doing a quick scan of the code for when it's set and cleared. Anyway, looks good. Reviewed-by: Alex Elder

Re: [PATCH] rbd: fix double free on rbd_dev->header_name

arent images). > > rbd_dev_probe_parent() is responsible for probing the parent, so it > shoudn't mock with clone's fields. Agreed. (But I think you mean "muck with.") The other argument is that the caller is who allocated it (via rbd_dev_header_name()), so it should be responsible for freei

Re: [PATCH 01/18] libceph: add scatterlist messenger data type

On 07/29/2015 04:23 AM, mchri...@redhat.com wrote: From: Mike Christie micha...@cs.wisc.edu LIO uses scatterlist for its page/data management. This patch adds a scatterlist messenger data type, so LIO can pass its sg down directly to rbd. Signed-off-by: Mike Christie micha...@cs.wisc.edu

Re: [PATCH] rbd: fix copyup completion race

... Reviewed-by: Alex Elder el...@linaro.org Cc: Alex Elder el...@linaro.org Cc: sta...@vger.kernel.org # 3.10+, needs backporting for 3.18 Signed-off-by: Ilya Dryomov idryo...@gmail.com --- drivers/block/rbd.c | 20 1 file changed, 16 insertions(+), 4 deletions

Re: [PATCH] rbd: use GFP_NOIO in rbd_obj_request_create()

a) this is going to stable, and b) those callers shouldn't really use rbd_obj_request_create() and will be fixed in the future. More memory allocation fixes will follow. This looks OK to me. It's conservative; you can add a GFP parameter in the future. Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH] libceph: Fix ceph_tcp_sendpage()'s more boolean usage

more) The logic is inverted: correct it. Whoops. I'm surprised we haven't had more trouble from this. This should go to stable too. Looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Benoît Canet benoit.ca...@nodalink.com --- net/ceph/messenger.c | 2 +- 1 file changed, 1

Re: [PATCH] rbd: bump queue_max_segments

for something like $ dd if=/dev/zero of=/dev/rbd0 oflag=direct bs=$RBD_OBJ_SIZE $ dd if=/dev/rbd0 iflag=direct of=/dev/null bs=$RBD_OBJ_SIZE Signed-off-by: Ilya Dryomov idryo...@gmail.com This looks good. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 1 + 1 file

Re: [PATCH 3/3] rbd: queue_depth map option

-by: Ilya Dryomov idryo...@gmail.com I haven't gone to follow through what happens with this but I assume a value that's too large will be caught when it's attempted to be used or something. In any case this looks good to me. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 17

Re: [PATCH] Avoid holding the zero page on slab init failure

On 06/24/2015 08:27 PM, Benoît Canet wrote: Spotted by visual inspection. Applies on libceph: Remove spurious kunmap() of the zero page. Benoît Canet (1): libceph: Avoid holding the zero page on ceph_msgr_slab_init errors net/ceph/messenger.c | 10 +- 1 file changed, 5

Re: [PATCH 1/3] rbd: terminate rbd_opts_tokens with Opt_err

On 06/25/2015 04:11 AM, Ilya Dryomov wrote: Also nuke useless Opt_last_bool and don't break lines unnecessarily. Signed-off-by: Ilya Dryomov idryo...@gmail.com Good idea. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 24 1 file changed, 8

Re: [PATCH 2/3] rbd: store rbd_options in rbd_device

On 06/25/2015 04:11 AM, Ilya Dryomov wrote: Signed-off-by: Ilya Dryomov idryo...@gmail.com Now that you need it when initializing the disk, this makes sense. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 18 +++--- 1 file changed, 11 insertions(+), 7

Re: [PATCH] libceph: Avoid holding the zero page on ceph_msgr_slab_init errors

ordering (tear down in reverse order of set up). Reviewed-by: Alex Elder el...@linaro.org BUG_ON() will not suffer to be postponed in case it is triggered. This is unrelated, but in cases like this (where there's already something in place to return an error) we should replace these BUG_ON() calls

Re: [PATCH] remove unbalanced kunmap

On 06/24/2015 07:35 PM, Alex Elder wrote: On 06/24/2015 04:18 PM, Benoît Canet wrote: Spotted via visual inspection of the code. Benoît Canet (1): libceph: Remove spurious kunmap() of the zero page net/ceph/messenger.c | 1 - 1 file changed, 1 deletion(-) I got no patch

Re: [PATCH] remove unbalanced kunmap

On 06/24/2015 04:18 PM, Benoît Canet wrote: Spotted via visual inspection of the code. Benoît Canet (1): libceph: Remove spurious kunmap() of the zero page net/ceph/messenger.c | 1 - 1 file changed, 1 deletion(-) I got no patch with this. Is it just me? -Alex -- To unsubscribe

Re: [PATCH] libceph: Remove spurious kunmap() of the zero page

On 06/24/2015 04:18 PM, Benoît Canet wrote: ceph_tcp_sendpage already does the work of mapping/unmapping the zero page if needed. Signed-off-by: Benoît Canet benoit.ca...@nodalink.com This looks good. Reviewed-by: Alex Elder el...@linaro.org --- net/ceph/messenger.c | 1 - 1 file

Re: [PATCH 3/5] libceph: a couple tweaks for wait loops

On 05/25/2015 05:38 AM, Ilya Dryomov wrote: On Thu, May 21, 2015 at 4:29 PM, Alex Elder el...@ieee.org wrote: On 05/21/2015 07:35 AM, Ilya Dryomov wrote: - return -ETIMEDOUT instead of -EIO in case of timeout - wait_event_interruptible_timeout() returns time left until timeout and since

Re: [PATCH v4 08/11] block: kill merge_bvec_fn() completely

On 05/25/2015 10:02 AM, Ilya Dryomov wrote: On Mon, May 25, 2015 at 5:04 PM, Christoph Hellwig h...@lst.de wrote: On Fri, May 22, 2015 at 11:18:40AM -0700, Ming Lin wrote: From: Kent Overstreet kent.overstr...@gmail.com As generic_make_request() is now able to handle arbitrarily sized bios,

Re: [PATCH 1/5] libceph: nuke time_sub()

On 05/21/2015 07:35 AM, Ilya Dryomov wrote: Unused since ceph got merged into mainline I guess. Signed-off-by: Ilya Dryomov idryo...@gmail.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- include/linux/ceph/libceph.h | 9 - 1 file changed, 9 deletions(-) diff

Re: [PATCH 2/5] libceph: store timeouts in jiffies, verify user input

good. I like your use of local variables for the options pointer; it makes it easy to see in the code where the timeout values come from. You could have handled timeout option checking and error reporting generically, but I'm not sure that would be better. Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH 3/5] libceph: a couple tweaks for wait loops

, this looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov idryo...@gmail.com --- net/ceph/ceph_common.c | 7 +++ net/ceph/mon_client.c | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c

Re: [PATCH v2 5/5] rbd: timeout watch teardown on unmap with mount_timeout

resource agent, for example) then have to worry about setting up their own timeouts. Timeout it with mount_timeout (60 seconds by default). Signed-off-by: Ilya Dryomov idryo...@gmail.com Reviewed-by: Sage Weil s...@redhat.com You can now add: Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH 4/5] ceph: simplify two mount_timeout sites

On 05/21/2015 07:35 AM, Ilya Dryomov wrote: No need to bifurcate wait now that we've got ceph_timeout_jiffies(). Looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov idryo...@gmail.com --- fs/ceph/dir.c| 14 -- fs/ceph/mds_client.c | 18

Re: [PATCH 02/10] ceph: add start/finish encoding helpers

On 05/01/2015 02:47 PM, Mike Christie wrote: On 05/01/2015 02:39 PM, Mike Christie wrote: On 04/30/2015 07:22 AM, Alex Elder wrote: +/** + * ceph_start_decoding_compat - decode block with legacy support for older schemes + * @p: buffer to decode + * @end: end of decode buffer + * @curr_ver

Re: [PATCH 1/3] libceph: properly release STAT request's raw_data_in

On 04/27/2015 03:24 AM, Yan, Zheng wrote: Signed-off-by: Yan, Zheng z...@redhat.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- net/ceph/osd_client.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index 41a4abc..b93531f

[PATCH 0/7] ceph-client: some refactoring patches

is based on the current testing branch: 5eeb14f ceph: check OSD caps before read/write -Alex Alex Elder (7): messenger: update state diagram messenger: add some clarifying comments messenger: rename out_kvec_left field osd_client: don't supply

[PATCH 1/7] messenger: update state diagram

Update the fabulous state diagram in messenger.c to reflect the way the code operates. Two new transitions are added: CLOSED - CLOSED CLOSING - CLOSING Signed-off-by: Alex Elder el...@linaro.org --- net/ceph/messenger.c | 51 +-- 1 file

[PATCH 7/7] messenger: con_work() is not really a loop

so it's *not* a loop, and use goto calls rather than a loop to implement the control flow. Signed-off-by: Alex Elder el...@linaro.org --- net/ceph/messenger.c | 84 +--- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/net/ceph

[PATCH 6/7] messenger: encapsulate grabbing incoming message

an error, then drop and ignore the message. Take care to use the right connection's put operation. Establish the naming convention that in_msg is a pointer to a message that was a connection's incoming message. (This affects some additional code in process_message().) Signed-off-by: Alex Elder el

[PATCH 2/7] messenger: add some clarifying comments

that as well. Signed-off-by: Alex Elder el...@linaro.org --- net/ceph/messenger.c | 9 + 1 file changed, 9 insertions(+) diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 9dfdb20..37b0fa7 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -2733,6 +2733,10 @@ static

[PATCH 3/7] messenger: rename out_kvec_left field

to out_kvec_used to avoid misunderstanding. Signed-off-by: Alex Elder el...@linaro.org --- include/linux/ceph/messenger.h | 2 +- net/ceph/messenger.c | 22 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph

[PATCH 4/7] osd_client: don't supply connection to handle_reply()

The connection parameter in handle_reply() is never used, so get rid of it. Signed-off-by: Alex Elder el...@linaro.org --- net/ceph/osd_client.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index e3c2e8b..6539dfc 100644

Re: [PATCHv2 0/3] rbd: header read/refresh improvements

On 04/24/2015 08:22 AM, Douglas Fuller wrote: Support multiple class op calls in one ceph_msg and consolidate rbd header read and refresh processes to use this feature to reduce the number of ceph_msgs sent for that process. Refresh features on header refresh and begin returning EIO if features

Re: [PATCHv2 0/3] rbd: header read/refresh improvements

On 04/26/2015 11:35 PM, Douglas Fuller wrote: This solution just feels hacky and inefficient, so I think there is a desire to feel like we at least tried to come up something simpler and more efficient before proceeding. Right. And somehow more fitting with the existing code, if that's

Re: [PATCH 0/3] rbd: header read/refresh improvements

On 04/23/2015 02:06 PM, Douglas Fuller wrote: Support multiple class op calls in one ceph_msg and consolidate rbd header read and refresh processes to use this feature to reduce the number of ceph_msgs sent for that process. Refresh features on header refresh and begin returning EIO if features

Re: [PATCH 0/3] rbd: header read/refresh improvements

On 04/24/2015 09:40 AM, Douglas Fuller wrote: On Apr 24, 2015, at 10:17 AM, Ilya Dryomov idryo...@gmail.com wrote: On Fri, Apr 24, 2015 at 4:11 PM, Alex Elder el...@ieee.org wrote: On 04/23/2015 02:06 PM, Douglas Fuller wrote: Support multiple class op calls in one ceph_msg and consolidate

Re: [PATCH] libceph: kfree() in put_osd() shouldn't depend on authorizer

nevertheless. The fix looks good. Reviewed-by: Alex Elder el...@linaro.org Cc: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov idryo...@gmail.com --- net/ceph/osd_client.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c

Re: [PATCH] libceph: fix double __remove_osd() problem

, that sounds like refcounting isn't working as desired... The mutex around all calls to (now) remove_osd() avoids races. I like the RB_CLEAR_NODE() call anyway. OK, enough chit chat. This looks OK to me. Reviewed-by: Alex Elder el...@linaro.org Cc: Sage Weil sw...@redhat.com Cc: sta...@vger.kernel.org

Re: [PATCH 1/3] rbd: fix rbd_dev_parent_get() when parent_overlap == 0

) performance regressions with this change in place... Reviewed-by: Alex Elder el...@linaro.org Cc: sta...@vger.kernel.org # 3.11+ Signed-off-by: Ilya Dryomov idryo...@redhat.com --- drivers/block/rbd.c | 20 ++-- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git

Re: [PATCH 3/3] rbd: do not treat standalone as flatten

you're doing and it looks OK to me. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 30 ++ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index b85d52005a21..e818c2a6ffb1 100644

Re: [PATCH 2/3] rbd: drop parent_ref in rbd_dev_unprobe() unconditionally

pointer rather than non-zero parent_overlap. And that's done inside rbd_dev_parent_put(), so your change looks reasonable to me. Reviewed-by: Alex Elder el...@linaro.org My thinking behind calling rbd_dev_parent_put() unconditionally is that there shouldn't be any requests in flight at that point

Re: [PATCH] rbd: convert to blk-mq

two warnings about endo-of-line whitespace in your patch. And I have one other very small suggestion below. Other than those things, this looks great to me. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/block/rbd.c | 118

Re: [PATCH] rbd: don't treat CEPH_OSD_OP_DELETE as extent op

On 11/24/2014 03:59 AM, Ilya Dryomov wrote: CEPH_OSD_OP_DELETE is not an extent op, stop treating it as such. This sneaked in with discard patches - it's one of the three osd ops (the other two are CEPH_OSD_OP_TRUNCATE and CEPH_OSD_OP_ZERO) that discard is implemented with. Signed-off-by:

Re: krbd blk-mq support ?

On 10/28/2014 01:07 PM, Christoph Hellwig wrote: On Mon, Oct 27, 2014 at 11:00:46AM +0100, Alexandre DERUMIER wrote: Can you do a perf report -ag and then a perf report to see where these cycles are spent? Yes, sure. I have attached the perf report to this mail. (This is with kernel 3.14,

Re: krbd blk-mq support ?

On 10/28/2014 01:07 PM, Christoph Hellwig wrote: On Mon, Oct 27, 2014 at 11:00:46AM +0100, Alexandre DERUMIER wrote: Can you do a perf report -ag and then a perf report to see where these cycles are spent? Yes, sure. I have attached the perf report to this mail. (This is with kernel 3.14,

Re: [PATCH 2/2] libceph: resend lingering requests with a new tid

intend to do, and if you are sure my big question is not an issue you can go ahead and add this: Reviewed-by: Alex Elder el...@linaro.org If not, please update and give me a chance to look at it again. Thanks. -Alex Signed-off-by: Ilya Dryomov

Re: [PATCH] libceph: fix a memory leak in handle_watch_notify

On 09/11/2014 03:31 AM, Ilya Dryomov wrote: On Thu, Sep 11, 2014 at 5:41 AM, Alex Elder el...@ieee.org wrote: On 09/10/2014 07:20 PM, roy.qing...@gmail.com wrote: From: Li RongQing roy.qing...@gmail.com event_work should be freed when adding it to queue failed Signed-off-by: Li RongQing

Re: [PATCH] rbd: do not return -ERANGE on auth failure

code that was expected by a write() call would be returned. In any case, this looks good to me. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index

Re: [PATCH] rbd: do not return -ERANGE on auth failure

On 09/11/2014 11:17 AM, Ilya Dryomov wrote: On Thu, Sep 11, 2014 at 7:23 PM, Alex Elder el...@ieee.org wrote: On 09/11/2014 10:10 AM, Ilya Dryomov wrote: Trying to map an image out of a pool for which we don't have an 'x' permission bit fails with -ERANGE from ceph_extract_encoded_string

Re: [PATCH] rbd: do not return -ERANGE on auth failure

On 09/11/2014 11:27 AM, Ilya Dryomov wrote: I should have asked this before. Why is a permission error leading to ceph_extract_encoded_string() finding a short buffer? I didn't take the time to trace the error path you're talking about here all the way back. rbd_obj_method_sync() returns

Re: [PATCH] rbd: do not return -ERANGE on auth failure

On 09/11/2014 11:17 AM, Ilya Dryomov wrote: On Thu, Sep 11, 2014 at 7:23 PM, Alex Elder el...@ieee.org wrote: On 09/11/2014 10:10 AM, Ilya Dryomov wrote: Trying to map an image out of a pool for which we don't have an 'x' permission bit fails with -ERANGE from ceph_extract_encoded_string

Re: [PATCH] libceph: fix a memory leak in handle_watch_notify

On 09/10/2014 07:20 PM, roy.qing...@gmail.com wrote: From: Li RongQing roy.qing...@gmail.com event_work should be freed when adding it to queue failed Signed-off-by: Li RongQing roy.qing...@gmail.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- net/ceph/osd_client.c |1

Re: [PATCH -next] rbd: fix error return code in rbd_dev_device_setup()

. Sorry I missed this when I first reviewed the workqueue code. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 623c841..f3be022 100644 --- a/drivers/block

Re: [PATCH] libceph: set last_piece in ceph_msg_data_pages_cursor_init() correctly

with more than one data item including a page array data item that was not last in the list would have problems. So in summary: Looks good. Reviewed-by: Alex Elder el...@linaro.org # cat pages-cursor-init.sh #!/bin/bash rbd create --size 10 --image-format 2 foo FOO_DEV=$(rbd map

Re: [PATCH] rbd: allocate img_request with GFP_NOIO instead GFP_ATOMIC

On 08/07/2014 04:40 AM, Ilya Dryomov wrote: Now that rbd_img_request_create() is called from work functions, no need to use GFP_ATOMIC. Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c |2 +- 1 file

Re: [PATCH] rbd: rework rbd_request_fn()

queue for all rbd devices, but I'm not sure that's very important. You'd have to stash the rbd_dev pointer in every request somewhere. Reviewed-by: Alex Elder el...@linaro.org Cc: sta...@vger.kernel.org # 3.15+ Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 195

Re: [PATCH] rbd: rework rbd_request_fn()

On 08/06/2014 02:09 PM, Ilya Dryomov wrote: Off topic... If you supply --patience to your git diff command you'll get an easier-to-read result in some cases (like this one). If you like that you can just do: git config --global --add diff.algorithm patience I'm generally using (and

Re: [PATCH 7/8] rbd: do not read in parent info before snap context

is unclear to me. I'll think about this and respond to your followup e-mail. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 64 --- 1 file changed, 30 insertions(+), 34

Re: [PATCH 7/8] rbd: do not read in parent info before snap context

On 07/25/2014 03:36 AM, Ilya Dryomov wrote: + if (ret) + return ret; I'll move this bit to harden rbd_dev_refresh() commit. Sounds good.-Alex -- To unsubscribe from this list: send the line unsubscribe ceph-devel in the body of a message to majord...@vger.kernel.org

Re: [PATCH] ceph: fix sizeof(struct tYpO *) typo

On 07/25/2014 04:26 AM, Ilya Dryomov wrote: struct ceph_xattr - struct ceph_inode_xattr Looks good. I can't find the definition of ceph_xattr. Reviewed-by: Alex Elder el...@linaro.org Reported-by: Toralf Förster toralf.foers...@gmx.de Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com

Re: [PATCH 6/8] rbd: update mapping size only on refresh

On 07/24/2014 10:10 AM, Ilya Dryomov wrote: On Thu, Jul 24, 2014 at 5:46 PM, Ilya Dryomov ilya.dryo...@inktank.com wrote: On Thu, Jul 24, 2014 at 5:25 PM, Alex Elder el...@ieee.org wrote: On 07/24/2014 03:42 AM, Ilya Dryomov wrote: There is no sense in trying to update the mapping size

Re: [PATCH] ceph: fix sizeof(struct tYpO *) typo

On 07/25/2014 08:27 AM, Ilya Dryomov wrote: On Fri, Jul 25, 2014 at 4:52 PM, Alex Elder el...@ieee.org wrote: On 07/25/2014 04:26 AM, Ilya Dryomov wrote: struct ceph_xattr - struct ceph_inode_xattr Looks good. I can't find the definition of ceph_xattr. That's the point of the patch

Re: [PATCH 1/8] rbd: show the entire chain of parent images

is the minimum supported version for the kernel (see Documentation/Changes). But fear not! That extension is supported in gcc 3.2: https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Conditionals.html#Conditionals Just FYI... Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo

Re: [PATCH 2/8] rbd: introduce rbd_dev_header_info()

On 07/24/2014 03:42 AM, Ilya Dryomov wrote: A wrapper around rbd_dev_v{1,2}_header_info() to reduce duplication. Looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 24 ++-- 1 file

Re: [PATCH 3/8] rbd: remove unnecessary asserts in rbd_dev_image_probe()

when confidence in certain things was lower. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 0d3be608f16f

Re: [PATCH 4/8] rbd: split rbd_dev_spec_update() into two functions

-by: Alex Elder el...@linaro.org --- drivers/block/rbd.c | 79 +++ 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 4541f6027e4a..23df1773ef77 100644 --- a/drivers/block/rbd.c

Re: [PATCH 5/8] rbd: harden rbd_dev_refresh() caller

On 07/24/2014 03:42 AM, Ilya Dryomov wrote: Recently discovered watch/notify problems showed that we really can't ignore errors in anything refresh related. Alas, currently there is not much we can do in response to those errors, except print warnings. Looks good. Reviewed-by: Alex Elder el

Re: [PATCH 6/8] rbd: update mapping size only on refresh

(or other) images. There's no need to update the mapping size for a snapshot--it'll never change. Is that right? If not, please advise; otherwise: Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 19

Re: [PATCH 6/8] rbd: update mapping size only on refresh

On 07/24/2014 08:46 AM, Ilya Dryomov wrote: On Thu, Jul 24, 2014 at 5:25 PM, Alex Elder el...@ieee.org wrote: On 07/24/2014 03:42 AM, Ilya Dryomov wrote: There is no sense in trying to update the mapping size before it's even been set. It took me a bit to follow this. But basically

Re: [PATCH 8/8] rbd: take snap_id into account when reading in parent info

2 images Parent images *must* be snapshots, so this was never right. I bet that was hard to figure out... Looks good. Reviewed-by: Alex Elder el...@linaro.org ret = rbd_obj_method_sync(rbd_dev, rbd_dev-header_name, rbd, get_parent

Re: [PATCH 10/14] rbd: rbd_obj_request_wait() should cancel the request if interrupted

On 07/08/2014 06:18 AM, Ilya Dryomov wrote: The only question that leaves me with is, does ceph_osdc_cancel_request() need to include the call to complete_request() that's present in ceph_osdc_wait_request()? I don't think so - I mentioned it in the ceph_osdc_cancel_request() function

Re: [PATCH 09/14] libceph: introduce ceph_osdc_cancel_request()

On 07/08/2014 06:15 AM, Ilya Dryomov wrote: Are you OK with your Reviewed-by for this patch? Reviewed-by: Alex Elder el...@linaro.org -- To unsubscribe from this list: send the line unsubscribe ceph-devel in the body of a message to majord...@vger.kernel.org More majordomo info at http

Re: [PATCH 09/14] libceph: introduce ceph_osdc_cancel_request()

On 06/30/2014 09:34 AM, Ilya Dryomov wrote: On Mon, Jun 30, 2014 at 5:39 PM, Alex Elder el...@ieee.org wrote: On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Introduce ceph_osdc_cancel_request() intended for canceling requests from the higher layers (rbd and cephfs). Because higher layers

Re: [PATCH 10/14] rbd: rbd_obj_request_wait() should cancel the request if interrupted

() that's present in ceph_osdc_wait_request()? I'll leave it at that. I think I've convinced myself this is a good change. So I await your confirmation that I understand it right, and your answer to my question above. But in any case you can consider this: Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH 13/14] libceph: nuke ceph_osdc_unregister_linger_request()

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Remove now unused ceph_osdc_unregister_linger_request(). Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- include/linux/ceph/osd_client.h |2 -- net/ceph/osd_client.c

Re: [PATCH 12/14] rbd: use rbd_obj_watch_request_helper() helper

different. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 115 --- 1 file changed, 17 insertions(+), 98 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c

Re: [PATCH 11/14] rbd: add rbd_obj_watch_request_helper() helper

. This commit cleanly abstracts the common bits, relying on the fixed rbd_obj_request_wait(). Adding this without calling it leads to an unused function warning in the build, I'm sure. You could probably squash this into the next patch. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya

Re: [PATCH 14/14] libceph: drop osd ref when canceling con work

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: queue_con() bumps osd ref count. We should do the reverse when canceling con work. Kind of unrelated to the rest of the series, but it looks good. Good to have a same-level-of-abstraction function for it as well. Reviewed-by: Alex Elder el

Re: [PATCH] rbd: do not leak image_id in rbd_dev_v2_parent_info()

On 06/30/2014 04:45 AM, Ilya Dryomov wrote: image_id is leaked if the parent happens to have been recorded already. Fix it. Looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c |2 ++ 1 file changed, 2

Re: [PATCH] rbd: handle parent_overlap on writes correctly

as it must be. Rounding up like you did for this test makes sense--it makes the granularity of this overlap test match the granularity of the exists test. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c | 10 +- 1 file

Re: [PATCH 01/14] libceph: rename ceph_osd_request::r_linger_osd to r_linger_osd_item

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: So that: req-r_osd_item -- osd-o_requests list req-r_linger_osd_item -- osd-o_linger_requests list Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com This looks good and I prefer it too. Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH 02/14] libceph: add maybe_move_osd_to_lru() and switch to it

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Abstract out __move_osd_to_lru() logic from __unregister_request() and __unregister_linger_request(). Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com Looks good. Reviewed-by: Alex Elder el...@linaro.org --- net/ceph/osd_client.c | 26

Re: [PATCH 03/14] libceph: move and add dout()s to ceph_msg_{get,put}()

: Reviewed-by: Alex Elder el...@linaro.org --- include/linux/ceph/messenger.h | 14 ++ net/ceph/messenger.c | 31 ++- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph

Re: [PATCH 04/14] libceph: move and add dout()s to ceph_osdc_request_{get,put}()

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Add dout()s to ceph_osdc_request_{get,put}(). Also move them to .c and turn kref release callback into a static function. You can pretty much take the identical comments from what I said on [PATCH 03/14]. Reviewed-by: Alex Elder el...@linaro.org

Re: [PATCH 06/14] libceph: assert both regular and lingering lists in __remove_osd()

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: It is important that both regular and lingering requests lists are empty when the OSD is removed. Looks good. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- net/ceph/osd_client.c |2 ++ 1

Re: [PATCH 05/14] libceph: harden ceph_osdc_request_release() a bit

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Add some WARN_ONs to alert us when we try to destroy requests that are still registered. Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com Good idea. Especially the RB_CLEAR_NODE() call. Reviewed-by: Alex Elder el...@linaro.org --- net

Re: [PATCH 07/14] libceph: unregister only registered linger requests

to linger but the request has completed successfully. Anyway, looks good. This explains why the rename of the r_linger_osd_item field was helpful. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- net/ceph/osd_client.c | 15

Re: [PATCH 08/14] libceph: fix linger request check in __unregister_request()

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: We should check if request is on the linger request list of any of the OSDs, not whether request is registered or not. Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com That was a difficult to spot bug. Very good. Reviewed-by: Alex Elder el

Re: [PATCH 09/14] libceph: introduce ceph_osdc_cancel_request()

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Introduce ceph_osdc_cancel_request() intended for canceling requests from the higher layers (rbd and cephfs). Because higher layers are in charge and are supposed to know what and when they are canceling, the request is not completed, only unref'ed

Re: [PATCH 07/14] libceph: unregister only registered linger requests

On 06/25/2014 12:16 PM, Ilya Dryomov wrote: Linger requests that have not yet been registered should not be unregistered by __unregister_linger_request(). This messes up ref count and leads to use-after-free. Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- net/ceph/osd_client.c

Re: [PATCH] rbd: fix ida/idr memory leak

On 06/04/2014 11:38 AM, Ilya Dryomov wrote: ida_destroy() needs to be called on module exit to release ida caches. Looks good to me. Reviewed-by: Alex Elder el...@linaro.org Signed-off-by: Ilya Dryomov ilya.dryo...@inktank.com --- drivers/block/rbd.c |1 + 1 file changed, 1 insertion

Re: [PATCH] ceph: refactor readpage_nounlock() to make the logic clearer

. Reviewed-by: Alex Elder el...@linaro.org --- fs/ceph/addr.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index b53278c..6aa2e3f 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -211,18 +211,15 @@ static int

Re: crash report: paging request errors in various krbd contexts

On 05/16/2014 01:00 AM, Hannes Landeholm wrote: A production server just locked up and had to be hard rebooted. It had these various rbd related crash signatures in it's system log within the same 10 second interval: I'll try to provide a quick summary of what's likely happened in each of

  1   2   3   4   5   6   7   8   9   10   >