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

AIX posix_fadvise and posix_fallocate #66590

Closed
DavidEdelsohn mannequin opened this issue Sep 12, 2014 · 13 comments
Closed

AIX posix_fadvise and posix_fallocate #66590

DavidEdelsohn mannequin opened this issue Sep 12, 2014 · 13 comments
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@DavidEdelsohn
Copy link
Mannequin

DavidEdelsohn mannequin commented Sep 12, 2014

BPO 22396
Nosy @pitrou, @vstinner, @serhiy-storchaka
Files
  • 10812_aix.patch
  • 22396_aix.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-09-30.10:41:09.471>
    created_at = <Date 2014-09-12.18:20:03.252>
    labels = ['type-bug', 'tests']
    title = 'AIX posix_fadvise and posix_fallocate'
    updated_at = <Date 2014-09-30.10:41:09.470>
    user = 'https://bugs.python.org/DavidEdelsohn'

    bugs.python.org fields:

    activity = <Date 2014-09-30.10:41:09.470>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-09-30.10:41:09.471>
    closer = 'vstinner'
    components = ['Tests']
    creation = <Date 2014-09-12.18:20:03.252>
    creator = 'David.Edelsohn'
    dependencies = []
    files = ['36611', '36699']
    hgrepos = []
    issue_num = 22396
    keywords = ['patch']
    message_count = 13.0
    messages = ['226834', '226836', '226838', '227293', '227316', '227317', '227355', '227356', '227357', '227358', '227359', '227875', '227876']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'python-dev', 'serhiy.storchaka', 'David.Edelsohn']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22396'
    versions = ['Python 3.3', 'Python 3.4', 'Python 3.5']

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Sep 12, 2014

    As with Solaris and bpo-10812, test_posix fadvise and fallocate fail on AIX. Python is compiled with _LARGE_FILES, which changes the function signature for posix_fadvise and posix_fallocate so that off_t is "long long" on 32 bit system passed in two registers. The Python call to those functions does not place the arguments in the correct registers, causing an EINVAL error. This patch fixes the failures in a similar way to Solaris ZFS kludge for bpo-10812.

    @DavidEdelsohn DavidEdelsohn mannequin added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Sep 12, 2014
    @pitrou
    Copy link
    Member

    pitrou commented Sep 12, 2014

    The Python call to those functions does not place the arguments in the correct registers

    Well... isn't there a way to fix this? I don't understand how this issue can come up.

    @serhiy-storchaka
    Copy link
    Member

    See similar Ruby issue: https://bugs.ruby-lang.org/issues/9914 .

    As workaround we can redeclare posix_fadvise as "int posix_fadvise(int fd, long offset, long len, int advice)" on 32-bit AIX with enabled _LARGE_FILES. More safe option is to disable posix_fadvise in such case (as Ruby had done).

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Sep 22, 2014

    Any feedback about which approach would be acceptable?

    @vstinner
    Copy link
    Member

    10812_aix.patch just hides the problem.

    I understand that AIX doesn't declare the function prototype correctly? I would prefer to disable the function in the posix module (don't declare it) if it's the case.

    @vstinner
    Copy link
    Member

    I understand that AIX doesn't declare the function prototype correctly?

    AIX bug report:
    http://www-01.ibm.com/support/docview.wss?uid=isg1IV56170

    I like Ruby's patch:

    -#ifdef HAVE_POSIX_FADVISE
    +    /* AIX currently does not support a 32-bit call to posix_fadvise()
    +     * if _LARGE_FILES is defined.
    +     */
    +#if defined(HAVE_POSIX_FADVISE) && !(defined(_AIX) && defined(_LARGE_FILES) && !defined(_ARCH_PPC64))

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Sep 23, 2014

    Attached is a revised patch that disables posix_fadvise() and posix_fallocate() when building on 32 bit AIX with _LARGE_FILES defined.

    @vstinner
    Copy link
    Member

    Attached is a revised patch that disables posix_fadvise() and posix_fallocate() when building on 32 bit AIX with _LARGE_FILES defined.

    Good. You should add a reference to this issue, something like "Issue bpo-22396: ...".

    To avoid code duplication, you may write something like:

    /* Issue python/cpython#66590: AIX currently does not support a 32-bit 
       call to posix_fallocate() if _LARGE_FILES is defined. */
    #if defined(HAVE_POSIX_FALLOCATE) && !(defined(_AIX) && defined(_LARGE_FILES) && !defined(__64BIT__))
    #  undef HAVE_POSIX_FALLOCATE
    #endif

    or "#define BROKEN_POSIX_FALLOCATE".

    Which Python versions should be patched? 3.4 and 3.5? Python 2.7 doesn't have the function (introduced in Python 3.3). For Python 3.4, it means that between two Python minor versions, the function disappears on AIX 32-bit :-/ Is it a problem since the function didn't work on this platform? (always fail with EINVAL)

    I suggest to patch 3.4 and 3.5.

    @serhiy-storchaka
    Copy link
    Member

    I think that some time AIX bug will be fixed. May be it would be better to introduce special macros, and set it in the configure script (similar to HAVE_GLIBC_MEMMOVE_BUG or HAVE_IPA_PURE_CONST_BUG). Or may be just udefine HAVE_POSIX_FADVISE at such circumstances.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 23, 2014

    Or can we simply keep the function and skip the test?

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Sep 23, 2014

    The declaration of the two system calls should be fixed in the AIX header, but the clueless response to the AIX problem report is underwhelming.

    I don't understand the "keep the function and skip the test" suggestion. I thought that was my first patch -- catch the exception of invalid argument and allow it to fail on AIX. If AIX eventually is fixed, the test will pass, no harm, no foul.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 30, 2014

    New changeset 8e5e19b3cd4e by Victor Stinner in branch '3.4':
    Issue bpo-22396: On 32-bit AIX platform, don't expose os.posix_fadvise() nor
    https://hg.python.org/cpython/rev/8e5e19b3cd4e

    New changeset 5ade1061fa3d by Victor Stinner in branch 'default':
    (Merge 3.4) Issue bpo-22396: On 32-bit AIX platform, don't expose
    https://hg.python.org/cpython/rev/5ade1061fa3d

    @vstinner
    Copy link
    Member

    Or can we simply keep the function and skip the test?

    Usually, we prefer to not provide the function in Python if it is known to be broken. Other examples:

    • HAVE_BROKEN_POLL: don't declare select.poll()
    • HAVE_BROKEN_PTHREAD_SIGMASK: don't declare signal.pthread_sigmask()

    There are also HAVE_BROKEN_NICE and HAVE_BROKEN_UNSETENV.

    Sorry, I'm too lazy to hack configure.ac, so I used a simple #ifdef in Modules/posixmodule.c to define "POSIX_FADVISE_AIX_BUG". If you feel more confortable with autotools, don't hesitate to propose a better patch :-)

    I consider that the issue is now fixed.

    @david: You should check with IBM why they don't fix the issue.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants