New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: client: reference counting 'struct Fh' #5222

Merged
1 commit merged into from Jul 29, 2015

Conversation

Projects
None yet
8 participants
@ukernel
Member

ukernel commented Jul 14, 2015

The async readahead finisher needs to reference 'struct Fh'. But
it's possible user closes FD and free the corresponding 'struct Fh'
before async readahead finishes.

Fixes: #12088
Signed-off-by: Yan, Zheng zyan@redhat.com
(cherry picked from commit 34b939a)

client: reference counting 'struct Fh'
The async readahead finisher needs to reference 'struct Fh'. But
it's possible user closes FD and free the corresponding 'struct Fh'
before async readahead finishes.

Fixes: #12088
Signed-off-by: Yan, Zheng <zyan@redhat.com>
(cherry picked from commit 34b939a)

@tchaikov tchaikov added the bug fix label Jul 14, 2015

@tchaikov tchaikov added this to the hammer milestone Jul 14, 2015

@shenyan1

This comment has been minimized.

shenyan1 commented Jul 14, 2015

Hi, Yan, I doubt that when thread1 block on read, thread2 attempt to close fd(call _release_fh) immediately. Can the problem happen again? Does thread1 use the closed "struct Fh"?

@ukernel

This comment has been minimized.

Member

ukernel commented Jul 14, 2015

On Tue, Jul 14, 2015 at 3:04 PM, Shen Yan notifications@github.com wrote:

Hi, Yan, I doubt that when thread1 block on read, thread2 attempt to close
fd(call _release_fh) immediately. Can the problem happen again? Does
thread1 use the closed "struct Fh"?

thread1 will access freed Fh in this case.


Reply to this email directly or view it on GitHub
#5222 (comment).

ghost pushed a commit that referenced this pull request Jul 15, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 15, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 16, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 19, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 19, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 19, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 19, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>

ghost pushed a commit that referenced this pull request Jul 19, 2015

Merge pull request #5222: client: reference counting 'struct Fh'
Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost

This comment has been minimized.

ghost commented Jul 28, 2015

@jcsp does this hammer backport looks good to merge ? It went through a fs suite ( http://tracker.ceph.com/issues/11990#fs ).

@jcsp

This comment has been minimized.

Contributor

jcsp commented Jul 29, 2015

@dachary : If @ukernel is happy with it then I am happy with it.

ghost pushed a commit that referenced this pull request Jul 29, 2015

Loic Dachary
Merge pull request #5222 from ceph/hammer-12088
client: reference counting 'struct Fh'

Reviewed-by: John Spray <john.spray@redhat.com>

@ghost ghost merged commit cbba706 into hammer Jul 29, 2015

@david-z

This comment has been minimized.

Member

david-z commented Jul 29, 2015

Hi guys,

There is an issue for this patch. Please check the details from this PR: #5311

Thanks.

@david-z

This comment has been minimized.

Member

david-z commented Jul 29, 2015

@dachary @ukernel , please take a look, thanks.

@ghost

This comment has been minimized.

ghost commented Jul 29, 2015

@david-z good catch, thanks ! IRC / mailed @jcsp to figure out if we should revert.

@david-z

This comment has been minimized.

Member

david-z commented Jul 30, 2015

@dachary

Yan had merged #5311 into hammer-12088 yesterday. You probably need to re-merge hammer-12088 into main hammer.

Thanks.

@ghost ghost changed the title from client: reference counting 'struct Fh' to fs: client: reference counting 'struct Fh' Aug 4, 2015

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment