Skip to content
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

Preemption Support #674

Closed
wants to merge 3 commits into from
Closed

Preemption Support #674

wants to merge 3 commits into from

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Apr 18, 2012

This reverts an earlier patch that causes a ./configure failure if CONFIG_PREEMPT and modifies txg_hold_open to support preemptable kernels. It depends on a pull request to zfsonlinux/spl and it closes issue #83.

@ryao
Copy link
Contributor Author

ryao commented Apr 19, 2012

I just spotted these in dmesg:

[ 1283.525520] BUG: using smp_processor_id() in preemptible [00000000] code: txg_sync/3247
[ 1283.525541] caller is fm_ena_generate+0x1b/0x40 [zfs]
[ 1283.525545] Pid: 3247, comm: txg_sync Tainted: P O 3.3.2 #18
[ 1283.525548] Call Trace:
[ 1283.525555] [] debug_smp_processor_id+0xdc/0x100
[ 1283.525571] [] fm_ena_generate+0x1b/0x40 [zfs]
[ 1283.525587] [] zfs_post_remove+0x6fc/0x730 [zfs]
[ 1283.525592] [] ? __switch_to+0xf4/0x410
[ 1283.525608] [] ? vdev_resilver_needed+0x158/0x160 [zfs]
[ 1283.525627] [] zfs_ereport_post+0x3a/0x60 [zfs]
[ 1283.525638] [] spa_event_notify+0x1d/0xb0 [zfs]
[ 1283.525649] [] dsl_prop_unset_hasrecvd+0xdd1/0x24d0 [zfs]
[ 1283.525660] [] dsl_sync_task_group_sync+0x123/0x3d0 [zfs]
[ 1283.525671] [] dsl_pool_sync+0x1eb/0x450 [zfs]
[ 1283.525682] [] spa_sync+0x38e/0xa00 [zfs]
[ 1283.525687] [] ? _raw_spin_unlock+0x15/0x40
[ 1283.525698] [] txg_sync_start+0x436/0x940 [zfs]
[ 1283.525710] [] ? txg_sync_start+0x1b0/0x940 [zfs]
[ 1283.525715] [] __thread_create+0x373/0x390 [spl]
[ 1283.525719] [] ? __thread_create+0x300/0x390 [spl]
[ 1283.525722] [] kthread+0x8e/0xa0
[ 1283.525726] [] kernel_thread_helper+0x4/0x10
[ 1283.525729] [] ? kthread_freezable_should_stop+0x60/0x60
[ 1283.525731] [] ? gs_change+0x13/0x13
[ 1514.623608] BUG: using smp_processor_id() in preemptible [00000000] code: txg_sync/3247
[ 1514.623631] caller is fm_ena_generate+0x1b/0x40 [zfs]
[ 1514.623634] Pid: 3247, comm: txg_sync Tainted: P O 3.3.2 #18
[ 1514.623637] Call Trace:
[ 1514.623643] [] debug_smp_processor_id+0xdc/0x100
[ 1514.623656] [] fm_ena_generate+0x1b/0x40 [zfs]
[ 1514.623668] [] zfs_post_remove+0x6fc/0x730 [zfs]
[ 1514.623682] [] ? vdev_dtl_reassess+0x374/0x390 [zfs]
[ 1514.623695] [] ? spa_debug_enabled+0x10/0xa0 [zfs]
[ 1514.623708] [] ? vdev_dtl_reassess+0x4c/0x390 [zfs]
[ 1514.623719] [] zfs_ereport_post+0x3a/0x60 [zfs]
[ 1514.623732] [] spa_event_notify+0x1d/0xb0 [zfs]
[ 1514.623745] [] dsl_prop_unset_hasrecvd+0xaab/0x24d0 [zfs]
[ 1514.623757] [] dsl_scan_sync+0x410/0x8d0 [zfs]
[ 1514.623770] [] spa_sync+0x3f1/0xa00 [zfs]
[ 1514.623785] [] txg_sync_start+0x436/0x940 [zfs]
[ 1514.623798] [] ? txg_sync_start+0x1b0/0x940 [zfs]
[ 1514.623803] [] __thread_create+0x373/0x390 [spl]
[ 1514.623809] [] ? __thread_create+0x300/0x390 [spl]
[ 1514.623814] [] kthread+0x8e/0xa0
[ 1514.623819] [] kernel_thread_helper+0x4/0x10
[ 1514.623823] [] ? kthread_freezable_should_stop+0x60/0x60
[ 1514.623827] [] ? gs_change+0x13/0x13

I am testing a fix for it. I will add it to this pull request after I have something that works.

@ryao ryao closed this Apr 23, 2012
@ryao ryao reopened this Apr 23, 2012
@ryao
Copy link
Contributor Author

ryao commented Apr 23, 2012

I accidentally clicked the close "Close & comment" button earlier.Anyway, I revised the txg_hold_open patch and also have a patch for fm_ena_generate. This should resolve the known issues in kernel preemption support.

@ryao
Copy link
Contributor Author

ryao commented Apr 28, 2012

It looks like we should be using get_cpu() and put_cpu() in situations where we use smp_processor_id(). I will rework these patches to use them when I find time.

@ierdnah
Copy link

ierdnah commented Jun 15, 2012

@ryao, could you please rebase on top of 0.6.0-rc9 ? Now it gives conflicts at merging.

@ryao
Copy link
Contributor Author

ryao commented Jun 17, 2012

@ierdnah, I have updated my patches, but I omitted changes to the ./configure script. People will need to run ./autogen.sh to regenerate the ./configure script until this is committed by Brian.

@ierdnah
Copy link

ierdnah commented Jun 18, 2012

@behlendorf could you please pull this in zfs ? This fix would give us a broader range of testers (those who have preemptible kernels).
@ryao: thx

@ierdnah
Copy link

ierdnah commented Jun 18, 2012

@ryao The rebase it's not ok, in your repo the last commit before your preempt patches is from 19 of april. If I try to merge your branch with 0.6.0-rc9 it gives conflicts at file config/kernel.m4.

@ryao
Copy link
Contributor Author

ryao commented Jun 18, 2012

@ierdnah Sorry about that. My tree should now be in sync.

@ierdnah
Copy link

ierdnah commented Jun 19, 2012

Thanks, it's ok now. I'm running zfs with a preemptible kernel.

@Nowaker
Copy link

Nowaker commented Jun 21, 2012

+1 for merging preempt "alpha" support, testers will come.

@ryao
Copy link
Contributor Author

ryao commented Jun 22, 2012

I started working on this after @kylef demonstrated to me with ryao/spl@7214cf4 that making this work was easier than I thought it would be. I want to finish this ASAP, but my time is very limited. I would be more than happy if a third person would volunteer to finish this work. Otherwise, it will remain in my to do list.

My issue list for this is that #757 should be examined (as full preemption in theory would only make that issue more likely) and get_cpu()/put_cpu() should be used. I believe Brian mentioned something about the SPL in another issue, although I am having trouble finding it.

@behlendorf Would you outline your concerns with regard to these patches? That way anyone interested in finishing this before I have a chance could do so.

On that note, a lightly patched version of 0.6.0-rc9 has entered Gentoo's testing tree. This effectively transfers control of what runs on users' systems from the ZFSOnLinux project to myself. Users have the option to override that, but few do. If Brian decides to merge these patches into the ZFSOnlinux project for wider testing, I will pull them into Gentoo. Otherwise, I will continue to wait until concerns blocking inclusion have been resolved, despite using these on my own system.

zfs_range_lock() is used in zvols, and previously, it could deadlock due
to an allocation using KM_SLEEP. We avoid this by moving responsibility
the memory allocation from zfs_range_lock() to the caller. This enables
us to avoid such deadlocks and use stack allocations, which are more
efficient and prevents deadlocks. The contexts in which stack
allocations are done do not appear to be stack heavy, so we do not risk
overflowing the stack from doing this.

Signed-off-by: Richard Yao <ryao@cs.stonybrook.edu>

Conflicts:

	module/zfs/zvol.c
@pyavdr
Copy link
Contributor

pyavdr commented Aug 1, 2012

Is there any unknown reason that these preemption support isn´t commited into ZOL? It may be not finished as the state of issue openzfs/spl#98 may indicate ... but it will be nice if there will be preemption support soon.

@behlendorf
Copy link
Contributor

It would be nice to have this done. But it's waiting on some minor reworking of openzfs/spl#98 and I still need to sit down and carefully review the needed zfs changes.

@ryao
Copy link
Contributor Author

ryao commented Aug 1, 2012

Issue #546 was something of a distraction from working on that. Anyway, I think I will work on openzfs/spl#98 as soon as openzfs/spl#147 is out of the way.

On that note, I need to remove ryao/zfs@513ad2a from this pull request. Further testing showed that the stack size was too small to accommodate it in all situations.

@ryao
Copy link
Contributor Author

ryao commented Aug 24, 2012

@prakash-surya has taken the time to improve upon these patches in pull request #891. This pull request is now obsolete.

@ryao ryao closed this Aug 24, 2012
behlendorf added a commit to behlendorf/zfs that referenced this pull request May 21, 2018
Add missing helper function cv_timedwait_io(), it should be used
when waiting on IO with a specified timeout.

Reviewed-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#674
pcd1193182 pushed a commit to pcd1193182/zfs that referenced this pull request Sep 26, 2023
Revert "zfs list: Allow more fields in ZFS_ITER_SIMPLE mode"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants