From: Alex Elder <elder@ieee.org>
To: Ilya Dryomov <idryomov@gmail.com>
Cc: Ceph Development <ceph-devel@vger.kernel.org>,
Zheng Yan <zyan@redhat.com>
Subject: Re: [PATCH 3/5] libceph: a couple tweaks for wait loops
Date: Mon, 25 May 2015 10:40:48 -0500 [thread overview]
Message-ID: <55634280.70309@ieee.org> (raw)
In-Reply-To: <CAOi1vP9JYRUfXv9hpGgVyBr0xsD6U4ocxMHRuiNEr=b+HTzzeg@mail.gmail.com>
On 05/25/2015 05:38 AM, Ilya Dryomov wrote:
> On Thu, May 21, 2015 at 4:29 PM, Alex Elder <elder@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 it can be almost LONG_MAX we had better assign it to long
>>
>> Any error returned by wait_event_interruptible_timeout()
>> can now be returned by __ceph_open_session(). It looks
>> like that may, in fact, be only -EINTR and -ERESTARTSYS.
>> But it's a change you could note in the log message.
>
> I think it's just -ERESTARTSYS so I didn't bother.
My point was almost a little more philosophical. It's conceivable
(though not likely) that wait_event_interruptible_timeout() could
be changed to return a value that your caller here does not expect.
>> It turns out the only caller ignores the return value of
>> ceph_monc_wait_osdmap() anyway. That should maybe be fixed.
>
> That's on purpose - rbd map tries to wait for a new enough osdmap only
> if the pool that the image is supposed to be in doesn't exist and we
> know we have a stale osdmap. We ignore wait retval because if we
> timeout we should return "this pool doesn't exist", not -ETIMEDOUT.
Yes I realize that. This second part of my response was
following on to my previous thought. That is, the caller
might get a different return value that it didn't expect;
but since the only caller ignores what gets returned, it's
a moot point.
-Alex
> Thanks,
>
> Ilya
>
next prev parent reply other threads:[~2015-05-25 15:40 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-21 12:35 [PATCH 0/5] libceph: timeout handling fixes Ilya Dryomov
2015-05-21 12:35 ` [PATCH 1/5] libceph: nuke time_sub() Ilya Dryomov
2015-05-21 12:40 ` Alex Elder
2015-05-21 12:35 ` [PATCH 2/5] libceph: store timeouts in jiffies, verify user input Ilya Dryomov
2015-05-21 12:58 ` Alex Elder
2015-05-21 12:35 ` [PATCH 3/5] libceph: a couple tweaks for wait loops Ilya Dryomov
2015-05-21 13:29 ` Alex Elder
2015-05-25 10:38 ` Ilya Dryomov
2015-05-25 15:40 ` Alex Elder [this message]
2015-05-21 12:35 ` [PATCH 4/5] ceph: simplify two mount_timeout sites Ilya Dryomov
2015-05-21 13:39 ` Alex Elder
2015-05-21 13:45 ` Yan, Zheng
2015-05-21 12:35 ` [PATCH v2 5/5] rbd: timeout watch teardown on unmap with mount_timeout Ilya Dryomov
2015-05-21 13:55 ` Alex Elder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55634280.70309@ieee.org \
--to=elder@ieee.org \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=zyan@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.