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

[RFC] Enable LFS by default on 32-bit #338

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Rondom
Copy link

@Rondom Rondom commented Feb 26, 2017

This uses a patch by Yury Norov that modifies the behaviour of the force_o_largefile() macro in the kernel. Previously O_LARGEFILE would be automatically enabled on 64-bit architectures only (and on 32-bit architectures one would have to set it manually). With the patch, the behaviour is controlled by a config-option CONFIG_32BIT_OFF_T that is only enabled for legacy 32-bit architectures. This way new 32-bit architectures like lkl can live without the 32-bit-file-offset-cruft.

I think this is very useful, especially because adding O_LARGEFILE is easy to forget, especially, when developing mostly on 64-bit.

The patch itself has been posted and acked by some people but has not found its way into mainline, yet. Last ping by Yury was here

I thought I would post this to hear your opinions.


This change is Reviewable

All new 32-bit architectures should have 64-bit off_t type, but existing
architectures has 32-bit ones.

To handle it, new config option is added to arch/Kconfig that defaults
ARCH_32BIT_OFF_T to be disabled for non-64 bit architectures. All existing
32-bit architectures enable it explicitly here.

New option affects force_o_largefile() behaviour. Namely, if off_t is
64-bits long, we have no reason to reject user to open big files.

Note that even if architectures has only 64-bit off_t in the kernel
(arc, c6x, h8300, hexagon, metag, nios2, openrisc, tile32 and unicore32),
a libc may use 32-bit off_t, and therefore want to limit the file size
to 4GB unless specified differently in the open flags.

Signed-off-by: Yury Norov <[email protected]>
@lkl-jenkins
Copy link

Can one of the admins verify this patch?

@Rondom Rondom closed this Feb 27, 2017
@Rondom Rondom deleted the lfs410 branch February 27, 2017 01:02
@Rondom Rondom restored the lfs410 branch February 27, 2017 01:03
@Rondom Rondom reopened this Feb 27, 2017
@lkl-jenkins
Copy link

Can one of the admins verify this patch?

Test that large file support is enabled by default, i.e. without passing
O_LARGEFILE to lkl_sys_open.

Signed-off-by: Andreas Gnau <[email protected]>
@tavip
Copy link
Member

tavip commented Mar 4, 2017

I would like to avoid touching other arch files so that we don't run into too many conflicts when merging with a new upstream version as sometimes event patches acked will not make it upstream.

Isn't it easier to define force_o_largefile in arch/lkl/include/uapi/asm/fcntl.h instead of using this patch?

@Rondom
Copy link
Author

Rondom commented Apr 1, 2017

I understand. Your reasoning makes sense.
Sorry, currently very busy. I will have a look at this later this month.

phhusson pushed a commit to phhusson/linux that referenced this pull request Feb 14, 2023
Currently amdgpu calls drm_sched_fini() from the fence driver sw fini
routine - such function is expected to be called only after the
respective init function - drm_sched_init() - was executed successfully.

Happens that we faced a driver probe failure in the Steam Deck
recently, and the function drm_sched_fini() was called even without
its counter-part had been previously called, causing the following oops:

amdgpu: probe of 0000:04:00.0 failed with error -110
BUG: kernel NULL pointer dereference, address: 0000000000000090
PGD 0 P4D 0
Oops: 0002 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 609 Comm: systemd-udevd Not tainted 6.2.0-rc3-gpiccoli lkl#338
Hardware name: Valve Jupiter/Jupiter, BIOS F7A0113 11/04/2022
RIP: 0010:drm_sched_fini+0x84/0xa0 [gpu_sched]
[...]
Call Trace:
 <TASK>
 amdgpu_fence_driver_sw_fini+0xc8/0xd0 [amdgpu]
 amdgpu_device_fini_sw+0x2b/0x3b0 [amdgpu]
 amdgpu_driver_release_kms+0x16/0x30 [amdgpu]
 devm_drm_dev_init_release+0x49/0x70
 [...]

To prevent that, check if the drm_sched was properly initialized for a
given ring before calling its fini counter-part.

Notice ideally we'd use sched.ready for that; such field is set as the latest
thing on drm_sched_init(). But amdgpu seems to "override" the meaning of such
field - in the above oops for example, it was a GFX ring causing the crash, and
the sched.ready field was set to true in the ring init routine, regardless of
the state of the DRM scheduler. Hence, we ended-up using sched.ops as per
Christian's suggestion [0], and also removed the no_scheduler check [1].

[0] https://lore.kernel.org/amd-gfx/[email protected]/
[1] https://lore.kernel.org/amd-gfx/[email protected]/

Fixes: 067f44c ("drm/amdgpu: avoid over-handle of fence driver fini in s3 test (v2)")
Suggested-by: Christian König <[email protected]>
Cc: Guchun Chen <[email protected]>
Cc: Luben Tuikov <[email protected]>
Cc: Mario Limonciello <[email protected]>
Reviewed-by: Luben Tuikov <[email protected]>
Signed-off-by: Guilherme G. Piccoli <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
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.

4 participants