Skip to content

Conversation

@backslashxx
Copy link
Contributor

No description provided.

@backslashxx backslashxx closed this by deleting the head repository Nov 19, 2025
Copy link
Member

@ThePedroo ThePedroo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though just small bits that require changes.

};

struct ksu_set_feature_cmd {
uint32_t feature_id; // Input: feature ID (enum ksu_feature_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the comment from this line and below to match the other structures.


ksu_uses_new_ksuctl = true;

struct ksu_set_feature_cmd cmd = {0};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct ksu_set_feature_cmd cmd = {
.feature_id = 1,
.value = 0
};

Instead

cmd.feature_id = (uint32_t)1;
cmd.value = (uint64_t)0;

/* we don't care about the reply */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* we don't care about the reply */
/* INFO: Regardless of its response, we follow the same path. Ignore response. */

@backslashxx
Copy link
Contributor Author

backslashxx commented Nov 19, 2025

oh, I thought you didnt like it, so I deleted the repo yesterday.
I'm too lazy to refork and redo it, You know how it works anyway, just re-add them yourself.

but this is something you might want to do now that ksu exposed umount,
tiann/KernelSU@4a18921

I'm not really up for fixing nitpicks when its essentially the same thing.

@ThePedroo
Copy link
Member

I'm not sure if we'll benefit much from its usage when the code for umount is unified -- Introducing support to have KSU handle that will not only limit future flexibility but also introduce additional code without any benefits.

I will be commiting this PR changes and credit you approprietly. Thank you. Next time, please don't rush to close it. I usually prefer to review in my PC, though this one you closed while I was reviewing on mobile LOL

@backslashxx
Copy link
Contributor Author

backslashxx commented Nov 20, 2025

no, the fucking point is that you can take over over kernelsu's kernel umount and properly announce it.
this also disables kernel based umount which is not great since it does not handle isoservice.

There is no benefit for rezygisk, the benefit is on the manager and its umount subsystem being disabled.

@ThePedroo
Copy link
Member

Please be respectful here. Yes, if you mean just its disable, indeed. However I was talking about the other features added in the refered commit, as this one has been already proven to be useful.

@backslashxx
Copy link
Contributor Author

oh the one on upstream removes hardcoded umount mountpoints and hands it to userspace for full control.
its just a linked list, for more context you can visit tiann/KernelSU#2950

ThePedroo added a commit that referenced this pull request Nov 21, 2025
This commit makes ReZygisk signal to KernelSU to not perform umount inside the kernel, as ReZygisk will already handle the umount process. Original code from PR #254.

Co-Authored-By: backslashxx <[email protected]>
ThePedroo added a commit that referenced this pull request Nov 21, 2025
This commit makes ReZygisk signal to KernelSU to not perform umount inside the kernel, as ReZygisk will already handle the umount process. Original code from PR #254.

Co-Authored-By: backslashxx <[email protected]>
Keyaku pushed a commit to Keyaku/ReZygisk that referenced this pull request Nov 25, 2025
This commit makes ReZygisk signal to KernelSU to not perform umount inside the kernel, as ReZygisk will already handle the umount process. Original code from PR PerformanC#254.

Co-Authored-By: backslashxx <[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.

2 participants